From owner-freebsd-fs@freebsd.org Tue Aug 22 19:28:32 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 236D6DDE77A for ; Tue, 22 Aug 2017 19:28:32 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: from mail-oi0-f43.google.com (mail-oi0-f43.google.com [209.85.218.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id E419771BA0 for ; Tue, 22 Aug 2017 19:28:31 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: by mail-oi0-f43.google.com with SMTP id j144so68137003oib.1 for ; Tue, 22 Aug 2017 12:28:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:reply-to:in-reply-to:references :from:date:message-id:subject:to; bh=IImXRiawThY9/iH8Hz20LkYtc5x8446sg5IFl6BvjjI=; b=sLw9JtLfFDNyDcHr0No0+yldMNpT2O+vjNoRdu9yR2/gZKYVsL7kUSY/rwuDQsnRQB nspNC3basoIumw4y4goah3yBgePex5Xwp3mhH70tkd8X9Nqpmwr4CWWgOP0tTWFxDM+1 YVrHwjERtDRvpYUUTdQOi5GHa5CjwrYqY1lOWkhpJChEC+DWLqyyn5I1YgYL0BJm1rBs 8lhZMGXJ/GCHpFcmhvOzJjW1b8tNWdCFsusDnLB6LBaj4PluFkFUuab7/rw2wUyUL/dw jNx1jHqn7RZdGJDBQqTEyuNhurw8qkVsP60pqrrVsUg/i9ULxmPvOoDsOqG91M9V6L41 aVHQ== X-Gm-Message-State: AHYfb5iNoFbyENH7WrNWOJlzzrrqaCba6n3A2hbTI8hinV0XsfG4FRt9 +qZd8dL8bZJ3avF7H44= X-Received: by 10.202.196.213 with SMTP id u204mr214495oif.318.1503428730626; Tue, 22 Aug 2017 12:05:30 -0700 (PDT) Received: from mail-it0-f46.google.com (mail-it0-f46.google.com. [209.85.214.46]) by smtp.gmail.com with ESMTPSA id s62sm127421oie.16.2017.08.22.12.05.30 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 22 Aug 2017 12:05:30 -0700 (PDT) Received: by mail-it0-f46.google.com with SMTP id o19so287768ito.0 for ; Tue, 22 Aug 2017 12:05:30 -0700 (PDT) X-Received: by 10.36.129.138 with SMTP id q132mr838238itd.174.1503428729893; Tue, 22 Aug 2017 12:05:29 -0700 (PDT) MIME-Version: 1.0 Reply-To: cem@freebsd.org Received: by 10.2.81.131 with HTTP; Tue, 22 Aug 2017 12:05:29 -0700 (PDT) In-Reply-To: References: <87bmn7kf3b.fsf@vostro.rath.org> <87lgmbfob2.fsf@vostro.rath.org> From: Conrad Meyer Date: Tue, 22 Aug 2017 12:05:29 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: Can telldir() == 0 value be made special? To: "freebsd-fs@freebsd.org" Content-Type: text/plain; charset="UTF-8" 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: Tue, 22 Aug 2017 19:28:32 -0000 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.