From patchwork Tue Mar 1 22:41:10 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Eggert X-Patchwork-Id: 11162 Received: (qmail 105668 invoked by alias); 1 Mar 2016 22:41:16 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 105649 invoked by uid 89); 1 Mar 2016 22:41:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=327, 32, 7, direnth, dirent64 X-HELO: zimbra.cs.ucla.edu Subject: Re: [PATCH] Fix readdir_r with long file names To: Florian Weimer , "Michael Kerrisk (man-pages)" , Siddhesh Poyarekar References: <51B0B39F.4060202@redhat.com> <51B0BD36.3030202@redhat.com> <20130607013024.GO29800@brightrain.aerifal.cx> <51B19203.3070307@redhat.com> <20130607144143.GQ29800@brightrain.aerifal.cx> <51B57E35.4080403@redhat.com> <51B65EA7.2020402@redhat.com> <20130611011324.GT29800@brightrain.aerifal.cx> <51B8702D.2060505@redhat.com> <20130813040038.GE21795@spoyarek.pnq.redhat.com> <520C88A6.9070501@redhat.com> <56D54DAD.1040306@gmail.com> <56D5CA79.9030204@redhat.com> <56D5F832.3070209@gmail.com> <56D5FB3D.5000306@redhat.com> <56D607BB.6080701@cs.ucla.edu> <56D614AA.7020500@redhat.com> Cc: Rich Felker , Carlos O'Donell , KOSAKI Motohiro , libc-alpha , Roland McGrath , linux-man From: Paul Eggert Message-ID: <56D61A86.3050108@cs.ucla.edu> Date: Tue, 1 Mar 2016 14:41:10 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <56D614AA.7020500@redhat.com> On 03/01/2016 02:16 PM, Florian Weimer wrote: >> Why not use a flexible array member for this? > For which part, and how exactly? Something like the attached patch, say. (Totally untested.) > You can't put a flexible array member into a transparent union. That's OK. Any such usage of struct dirent would be unportable anyway. > If you mean to add some zero-width padding member at the end of the > struct, after the d_name member, then I'm worried that makes overrunning > the d_name array member even more undefined than it already is. No, no padding member, just use C99 the way it was designed. This should improve overrun detection in programs like valgrind. With glibc's current definition these programs can be fooled into thinking that struct dirent accesses are invalid (outside of array bounds) when they are actually OK, so people shut off array-bounds checking. If we used flexible array members, valgrind etc. should know that the array's upper bound is unknown and should not issue so many false alarms, so people can leave bounds checking on. Also, I expect this sort of thing will become more important as GCC -fbounds-check becomes more practical. If flexible arrays are no-go for some reason, I suppose we could use 'char 'd_name[SIZE_MAX - 1000];' instead. That should get peoples' attention. :-) diff --git a/bits/dirent.h b/bits/dirent.h index 7b79a53..8546c29 100644 --- a/bits/dirent.h +++ b/bits/dirent.h @@ -32,7 +32,7 @@ struct dirent unsigned char d_namlen; /* Length of the file name. */ /* Only this member is in the POSIX standard. */ - char d_name[1]; /* File name (actually longer). */ + char d_name __flexarr; /* File name. */ }; #ifdef __USE_LARGEFILE64 @@ -42,8 +42,7 @@ struct dirent64 unsigned short int d_reclen; unsigned char d_type; unsigned char d_namlen; - - char d_name[1]; + char d_name __flexarr; /* File name. */ }; #endif