Can telldir() == 0 value be made special?

[Martin Simmons]

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 <lists at eitanadler.com> wrote:
> > On 22 August 2017 at 14:30, Nikolaus Rath <Nikolaus at rath.org> wrote:
> >> On Aug 22 2017, Conrad Meyer <cem at freebsd.org> 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 at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-fs
> To unsubscribe, send any mail to "freebsd-fs-unsubscribe at freebsd.org"
>