From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id AECEC385771F for ; Wed, 5 Jul 2023 20:31:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AECEC385771F Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1688589115; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=t/+Lz3myPLcPHra2ZJQAuAhy0ta0QGiAzk23fVlIWbs=; b=OAlpmJGloTbvxvXd968bbFsI9ceI+giQopDYQ6zs38UlJrjBYyWtDKH1KQIK/Y1MasVMlU naDwaZdPvgoLfOPcQ862f2noJ6bccom08Eb62QhhH26NFDS/9n7PZJ4T+fvD1qN89UD30l t7F0fm36w06nd89KlmOGe1lhaziFrFo= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-82-2MKk1vtANFKyOYwPEK9WSw-1; Wed, 05 Jul 2023 16:31:54 -0400 X-MC-Unique: 2MKk1vtANFKyOYwPEK9WSw-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id CD8D2185A78B for ; Wed, 5 Jul 2023 20:31:53 +0000 (UTC) Received: from oak (unknown [10.22.8.210]) by smtp.corp.redhat.com (Postfix) with ESMTPS id B05942166B25; Wed, 5 Jul 2023 20:31:53 +0000 (UTC) Date: Wed, 5 Jul 2023 16:31:52 -0400 From: Joe Simmons-Talbott To: DJ Delorie Cc: libc-alpha@sourceware.org Subject: Re: [PATCH v2] fileops: Don't process , ccs= as individual mode flags (BZ#18906) Message-ID: <20230705203152.GB6392@oak> References: <20230615171243.4173020-1-josimmon@redhat.com> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 3.1 on 10.11.54.6 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Wed, Jul 05, 2023 at 03:45:03PM -0400, DJ Delorie wrote: > > This patch looks semantically correct to me, but needs a few syntactical > changes to meet our coding style. Please post a v3 and I'll prioritize > re-reviewing it. Thanks! Thanks for reviewing it. I've posted a v3 with the changes you've suggested. Thanks, Joe > > Joe Simmons-Talbott via Libc-alpha writes: > > diff --git a/libio/fileops.c b/libio/fileops.c > > index 58c9e985e4..1c1113e339 100644 > > --- a/libio/fileops.c > > +++ b/libio/fileops.c > > @@ -247,6 +247,7 @@ _IO_new_file_fopen (FILE *fp, const char *filename, const char *mode, > > switch (*++mode) > > { > > case '\0': > > + case ',': > > break; > > case '+': > > omode = O_RDWR; > > While this does force the ,ccs= to be after any other flags, effectively > this was true before as the characters in "ccs" and/or the conversion > name would conflict anyway. Ok. > > > diff --git a/libio/tst-fopenloc.c b/libio/tst-fopenloc.c > > index 089c61bf41..8cd35f01f2 100644 > > --- a/libio/tst-fopenloc.c > > +++ b/libio/tst-fopenloc.c > > @@ -17,6 +17,7 @@ > > . */ > > > > #include > > +#include > > #include > > #include > > #include > > @@ -24,6 +25,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > > > @@ -48,13 +50,40 @@ do_bz17916 (void) > > if (fp != NULL) > > { > > printf ("unexpected success\n"); > > + free (ccs); > > + fclose (fp); > > return 1; > > } > > + > > free (ccs); > > > > return 0; > > } > > Ok. > > > +static int > > +do_bz18906 (void) > > +{ > > + /* BZ #18906 -- check processing of ,ccs= as flags case. */ > > + > > + const char *ccs = "r,ccs=+ISO-8859-1"; > > + size_t retval; > > + > > + FILE *fp = fopen (inputfile, ccs); > > + int flags; > > + > > + TEST_VERIFY(fp != NULL); > > + > > + if (fp != NULL) > > + { > > + flags = fcntl(fileno(fp), F_GETFL); > > + retval = (flags & O_RDWR) | (flags & O_WRONLY); > > + TEST_COMPARE(retval, false); > > + fclose (fp); > > + } > > + > > + return EXIT_SUCCESS; > > +} > > Ok. Needs space between "TEST_VERIFY" and "(" though, and > "TEST_COMPARE" and "(" > > Needs spaces in fcntl( and fileno( > > > @@ -78,7 +107,10 @@ do_test (void) > > > > xfclose (fp); > > > > - return do_bz17916 (); > > + TEST_COMPARE(do_bz17916 (), 0); > > + TEST_COMPARE(do_bz18906 (), 0); > > + > > + return EXIT_SUCCESS; > > } > > Ok, but same here - needs spaces before parens. >