From owner-freebsd-fs@freebsd.org Wed Aug 23 12:09:49 2017 Return-Path: Delivered-To: freebsd-fs@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 3B726DE58EC for ; Wed, 23 Aug 2017 12:09:49 +0000 (UTC) (envelope-from martin@lispworks.com) Received: from lwfs1-cam.cam.lispworks.com (mail.lispworks.com [46.17.166.21]) by mx1.freebsd.org (Postfix) with ESMTP id E25966D637 for ; Wed, 23 Aug 2017 12:09:47 +0000 (UTC) (envelope-from martin@lispworks.com) Received: from higson.cam.lispworks.com (higson.cam.lispworks.com [192.168.1.7]) by lwfs1-cam.cam.lispworks.com (8.15.2/8.15.2) with ESMTP id v7NC9c8D075146; Wed, 23 Aug 2017 13:09:38 +0100 (BST) (envelope-from martin@lispworks.com) Received: from higson.cam.lispworks.com (localhost.localdomain [127.0.0.1]) by higson.cam.lispworks.com (8.14.4) id v7NC9cSc008397; Wed, 23 Aug 2017 13:09:38 +0100 Received: (from martin@localhost) by higson.cam.lispworks.com (8.14.4/8.14.4/Submit) id v7NC9cYk008393; Wed, 23 Aug 2017 13:09:38 +0100 Date: Wed, 23 Aug 2017 13:09:38 +0100 Message-Id: <201708231209.v7NC9cYk008393@higson.cam.lispworks.com> From: Martin Simmons To: freebsd-fs@freebsd.org In-reply-to: (message from Conrad Meyer on Tue, 22 Aug 2017 12:05:29 -0700) Subject: Re: Can telldir() == 0 value be made special? References: <87bmn7kf3b.fsf@vostro.rath.org> <87lgmbfob2.fsf@vostro.rath.org> X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Aug 2017 12:09:49 -0000 Does this solution (and the previous one) need to check for dd_seek == 0 as well? It looks like dd_loc == 0 is also true after readdir has returned NULL for eof. __Martin >>>>> On Tue, 22 Aug 2017 12:05:29 -0700, Conrad Meyer said: > > One could also make it reserve the zero slot for the loc == 0 entry, > but not create it dynamically. Just set loccnt=1 at initialization > and then special-case dd_loc==0. Like this: > > --- a/lib/libc/gen/opendir.c > +++ b/lib/libc/gen/opendir.c > @@ -295,7 +295,7 @@ __opendir_common(int fd, int flags, bool use_current_pos) > dirp->dd_lock = NULL; > dirp->dd_td = (struct _telldir *)((char *)dirp + sizeof(DIR)); > LIST_INIT(&dirp->dd_td->td_locq); > - dirp->dd_td->td_loccnt = 0; > + dirp->dd_td->td_loccnt = 1; > dirp->dd_compat_de = NULL; > > /* > --- a/lib/libc/gen/telldir.c > +++ b/lib/libc/gen/telldir.c > @@ -76,7 +76,10 @@ telldir(DIR *dirp) > _pthread_mutex_unlock(&dirp->dd_lock); > return (-1); > } > - lp->loc_index = dirp->dd_td->td_loccnt++; > + if (dirp->dd_loc == 0) > + lp->loc_index = 0; > + else > + lp->loc_index = dirp->dd_td->td_loccnt++; > lp->loc_seek = dirp->dd_seek; > lp->loc_loc = dirp->dd_loc; > if (flp != NULL) > > Best, > Conrad > > > On Tue, Aug 22, 2017 at 11:35 AM, Eitan Adler wrote: > > On 22 August 2017 at 14:30, Nikolaus Rath wrote: > >> On Aug 22 2017, Conrad Meyer wrote: > >>> Hi Nikolaus, > >>> > >>> As you have surmised, DIR* seekpoints are created dynamically whenever > >>> requested by user's telldir() call: > >> > >> Good to know, thanks! > >> > >>> I believe we could special case zero without breaking ABI > >>> compatibility of correct programs. Something like this: > >>> > >>> --- a/lib/libc/gen/telldir.c > >>> +++ b/lib/libc/gen/telldir.c > >>> @@ -70,6 +70,20 @@ telldir(DIR *dirp) > >>> } > >>> } > >>> if (lp == NULL) { > >>> + /* Create special zero telldir entry, similar to Linux */ > >>> + if (dirp->dd_td->td_loccnt == 0 && dirp->dd_loc != 0) { > >>> + lp = malloc(sizeof(struct ddloc)); > >>> + if (lp == NULL) { > >>> + if (__isthreaded) > >>> + _pthread_mutex_unlock(&dirp->dd_lock); > >>> + return (-1); > >>> + } > >>> + lp->loc_index = dirp->dd_td->td_loccnt++; > >>> + lp->loc_seek = 0; > >>> + lp->loc_loc = 0; > >>> + LIST_INSERT_HEAD(&dirp->dd_td->td_locq, lp, loc_lqe); > >>> + } > >>> + > >>> lp = malloc(sizeof(struct ddloc)); > >>> if (lp == NULL) { > >>> if (__isthreaded) > >>> > >>> I don't know if there are any downsides to special-casing zero like > >>> this, other than additional code complexity. > >> > >> This seems a little more complex than I would have expected. > > > >> Is this > >> maybe turning zero into a valid argument for seekdir() even if telldir() > >> has never been called? > > > > No, this code is called inside of telldir. I'd be against adding that > > behavior by contract since it complicates the contract of seekdir. No > > objection to this behavior by implementation though if it is simpler > > than modifying telldir (doubtful). > > > > What this is doing is ensuring that there will always be a 0 entry > > *after* telldir is called, and that seekdir(0) will return to the > > start of the directory if called with an argument of 0. > > > > > >> That would be even better, but the minimal change that I need is just to > >> ensure that telldir() either (a) never returns zero, or (b) only returns > >> zero if called at the beginning of the stream. > _______________________________________________ > freebsd-fs@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/freebsd-fs > To unsubscribe, send any mail to "freebsd-fs-unsubscribe@freebsd.org" >