From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 89055 invoked by alias); 30 Sep 2015 11:01:15 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 89028 invoked by uid 89); 30 Sep 2015 11:01:14 -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,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 30 Sep 2015 11:01:13 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id F37C3AEF1D; Wed, 30 Sep 2015 11:01:11 +0000 (UTC) Received: from localhost (ovpn-116-138.ams2.redhat.com [10.36.116.138]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t8UB1A45016377; Wed, 30 Sep 2015 07:01:11 -0400 Date: Wed, 30 Sep 2015 12:00:00 -0000 From: Jonathan Wakely To: Martin Sebor Cc: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: Re: [patch] libstdc++/67747 Allocate space for dirent::d_name Message-ID: <20150930110110.GD12094@redhat.com> References: <20150929113726.GU12094@redhat.com> <560ADE6F.30104@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <560ADE6F.30104@gmail.com> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.5.23 (2014-03-12) X-SW-Source: 2015-09/txt/msg02321.txt.bz2 On 29/09/15 12:54 -0600, Martin Sebor wrote: >On 09/29/2015 05:37 AM, Jonathan Wakely wrote: >>POSIX says that dirent::d_name has an unspecified length, so calls to >>readdir_r must pass a buffer with enough trailing space for >>{NAME_MAX}+1 characters. I wasn't doing that, which works OK on >>GNU/Linux and BSD where d_name is a large array, but fails on Solaris >>32-bit. >> >>This uses pathconf to get NAME_MAX and allocates a buffer. >> >>Tested powerpc64le-linux and x86_64-dragonfly4.1, I'm going to commit >>this to trunk today (and backport all the filesystem fixes to >>gcc-5-branch). > >Calling pathconf is only necessary when _POSIX_NO_TRUNC is zero >which I think exists mainly for legacy file systems. Otherwise, >it's safe to use NAME_MAX instead. Avoiding the call to pathconf Oh, nice. I was using NAME_MAX originally but the glibc readdir_r(3) man-page has an example using pathconf and says that should be used, so I went down that route. >also avoids the TOCTOU between it and the call to opendir, and Also nice. >hardcoding the value makes it possible to avoid dynamically >allocating the dirent buffer. Can we be sure NAME_MAX will never be too big for the stack? As currently written _Dir::advance() can call itself recursively to skip the . and .. directories, so if we put the dirent buffer on the stack then maybe we should re-use the same one not create three large frames. >I didn't remember the MAX_PATH value on Windows anymore but from >what I've just read online it sounds like it's defined to 260. Yes, I found that value, but I think I found something saying the individual components were limited to 255. I'll make it 260 anyway. I think that might relate to UTF-16 characters, so we'd need a larger buffer for narrow characters, but I'm not sure what mingw supports here anyway. The Windows implementations are just stubs where someone who cares can add working code. >Defaulting to 255 on POSIX is appropriate. On XSI systems, the >minimum required value is _XOPEN_NAME_MAX which is 255 (I would >suggest using the macro instead when it's defined). Otherwise, >the strictly conforming minimum value would be 14 -- the value >of _POSIX_NAME_MAX, but since 255 is greater it's fine. > >Other than that, I tend to be leery of using plain char arrays >as buffers for objects of bigger types. I don't know to what >extent this is a problem for libstdc++ anymore as more and more >hardware is tolerant of misaligned accesses and as the default >new expression typically returns memory suitably aligned for >the largest fundamental type. But since there is no requirement >in the language that it do so and I would tend to err on the >side of caution and use operator new (as opposed to >new char[len]). For some reason I thought new char[len] would be suitably aligned for any type with sizeof <= len, but that's only true for operator new. (I should check I haven't made the same assumption elsewhere in the library!) std::aligned_union::type will give us what we need. >Martin > >PS I'm interpreting _POSIX_NO_TRUNC being zero as more >restrictive than if it was non-zero and so calling pathconf(p, >_PC_NO_TRUNC) should be required to also return non-zero for >such an implementation, regardless of p. But let me check that >I'm reading it right. >