From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.3 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 10273 invoked from network); 9 Mar 2022 04:38:26 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 9 Mar 2022 04:38:26 -0000 Received: (qmail 24361 invoked by uid 550); 9 Mar 2022 04:38:21 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 24324 invoked from network); 9 Mar 2022 04:38:20 -0000 X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=vVbqDIt+mQ+JDMWzHUUuLWF4yXWcFF88HDFjKaHWdX4=; b=VDZBtMOP/9j8ur3UkDD+qh7DPMKPl5ywi3Y9dLuDmz3jX1z0RdtQIY62uTCvw2Sghj xAa+K9ieYh0+QB+4RjDNXS6A6cFjo1xVTWxv2kz/CKLNF4ZOOYM0322saP6Lipnm57Hf 32XbF7oleOVXfSv7ulLQ6Va3Oue2KZeEwkUBqzJFvzT649DLVV2Ft2VKL6exW0CGJcMQ x5fPcyKu1J8h7MFRxbgvJyB5QoIHWmTE30AWQ/ZDEK92QAxd342ueAjd+fsmgM46fVU6 iS02oVApW4YavANSdjxEN7vlxjtC2taOHgr34Iclz7dc+owoHpLKalDgImiMzmwEq8dD ShDQ== X-Gm-Message-State: AOAM5314PT+PUMGLRXlCdJjaDO1NtrHNZhfOZcYTCjOEKW/SFDsOkh+0 as73e/cUZ92qkwaHYchAbHI3ZEa1qdikaa3VzlTtkdzuMgSOIf5K//6h7JWuS94l2WXKDWzZAlH gI/ssLtFaEpHE/++3XSI= X-Received: by 2002:a17:90b:3b43:b0:1bf:8b53:c695 with SMTP id ot3-20020a17090b3b4300b001bf8b53c695mr7246881pjb.96.1646800686801; Tue, 08 Mar 2022 20:38:06 -0800 (PST) X-Google-Smtp-Source: ABdhPJziWWfdKvy1Rsb8qz9xUj/4E2ALAhhcpkpBniUQKCq1CLVNci1r1K7VDEkl979kWNwZByIyBg== X-Received: by 2002:a17:90b:3b43:b0:1bf:8b53:c695 with SMTP id ot3-20020a17090b3b4300b001bf8b53c695mr7246858pjb.96.1646800686390; Tue, 08 Mar 2022 20:38:06 -0800 (PST) Date: Wed, 9 Mar 2022 13:37:53 +0900 From: Dominique MARTINET To: Ismael Luceno , musl@lists.openwall.com Cc: Rich Felker Message-ID: References: <20220123155955.16484-1-ismael@iodev.co.uk> <20220123155955.16484-3-ismael@iodev.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [musl] [PATCH v3 2/2] nftw: implement FTW_ACTIONRETVAL (GNU extension) Sorry for the delay, I was waiting for some "official" feedback and forgot about the patch as none came. Rich (or others), is there anything we can do to convince you the feature would be interesting? I had sent you recap from debian codesearch back in April last year: https://www.openwall.com/lists/musl/2021/04/12/10 but didn't hear any reply. It'd be great to just hear a final "no unless it gets more widely used" so I could give up and work on converting bpftools to not depend on it -- it's just frustrating due to the lack of reply, and while adding FTW_ACTIONRETVAL to musl is easy converting a user to something else is not so straightforward so I was putting it off... (Conversely, if there is interest please say so as well -- my patch had a couple of feedbacks I would be happy to address, but I just never sent a v2 because it looked like it wouldn't be accepted anyway from your (lack of) reaction. I don't care if my version of this one gets in ultimately, and can help either way.) Ismael, comments below. Ismael Luceno wrote on Mon, Jan 24, 2022 at 08:53:14PM +0100: > > > + if (!(flags & FTW_DEPTH) && (r=fn(path, &st, type, &lev))) { > > > + if ((flags & FTW_ACTIONRETVAL)) { > > > + if (r == FTW_STOP) return FTW_STOP; > > > + if (r == FTW_SKIP_SUBTREE) return 0; > > > + /* other values are saved for when returning */ > > > > Hm, I'd naively think you would want to return immediately the other > > values as well, so the else below is wrong? > > But I didn't take long enough to check what e.g. a SKIP_SIBLINGS would > > mean here, the construction just looks a bit odd to me. > > SKIP_SIBLINGS doesn't imply SKIP_SUBTREE, so must be saved and returned > when finishing. Hm, I was starting to agree with you and wondered how my patch worked, but actually... According to how glibc behaves, it does: ---- # in musl source tree after patch $ make -j6 make: Nothing to be done for 'all'. $ cat ftw/ftw.c #define _GNU_SOURCE #include #include #include int cb(const char *path, const struct stat *sb, int typeflag, struct FTW *ftwbuf) { printf("got %s\n", path); if (strcmp(path, "./src") == 0) return FTW_SKIP_SUBTREE; if (strcmp(path, "./.git") == 0) { return FTW_SKIP_SIBLINGS; } return 0; } int main() { int rc = nftw(".", cb, 10, FTW_ACTIONRETVAL); printf("rc %d\n", rc); return 0; } $ gcc -o ftw/ftw_glibc ftw/ftw.c $ gcc -o ftw/ftw_musl ftw/ftw.c lib/libc.a $ diff -u <( ./ftw/ftw_musl ) <( ./ftw/ftw_glibc ) --- /dev/fd/63 2022-03-09 13:22:55.566285599 +0900 +++ /dev/fd/62 2022-03-09 13:22:55.566285599 +0900 @@ -1,120 +1,3 @@ got . got ./.git -got ./.git/branches -got ./.git/hooks -got ./.git/hooks/applypatch-msg.sample -got ./.git/hooks/commit-msg.sample -got ./.git/hooks/fsmonitor-watchman.sample ... -got ./.git/ORIG_HEAD -got ./.git/index -got ./.git/HEAD rc 0 ---- so the glibc version of ftw skipped .git here when it only returned FTW_SKIP_SIBLINGS. You could argue that this is a bug in glibc, but since there is no spec for FTW_ACTIONRETVAL I had chosen at the time to implement something compatible bug-for-bug as "glibc is the reference". > > > path[l] = 0; > > > - if ((flags & FTW_DEPTH) && (r=fn(path, &st, type, &lev))) > > > - return r; > > > - > > > - return 0; > > > + if (flags & FTW_DEPTH) { > > > + r = fn(path, &st, type, &lev); > > > + /* ignore FTW_SKIP_SUBTREE (too late), the caller is broken */ > > > + if ((flags & FTW_ACTIONRETVAL) && r == FTW_SKIP_SUBTREE) > > > + return 0; > > > > IIRC the glibc implementation also ignores FTW_SKIP_SIBLINGS in that > > case (nftw returns 0), I'm not sure how much of a 1-to-1 implementation > > we want -- I had implemented my version through a black-box approach > > with a client exercising all kind of different code paths as for a gnu > > extension I'd assume glibc to be the reference. > > FTW_SKIP_SUBTREE makes no sense with FTW_DEPTH, but FTW_SKIP_SIBLINGS > works with both. I agree it makes no sense, this was another bug-for-bug thing: ---- $ cat subtree.c #define _GNU_SOURCE #include #include #include int cb(const char *path, const struct stat *sb, int typeflag, struct FTW *ftwbuf) { return FTW_SKIP_SIBLINGS; } int main() { int rc = nftw(".", cb, 10, FTW_ACTIONRETVAL); printf("rc %d\n", rc); return 0; } $ ./ftw_glibc rc 0 $ ./ftw_musl rc 3 ---- My version at the time just reset the return value at the end if FTW_ACTIONRETVAL was specified in flags, to conform to that. -- Dominique