From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id 87AEC394343F for ; Tue, 20 Oct 2020 12:35:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 87AEC394343F Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-456-7tVmVpDnOtmNsPu2NN0UrQ-1; Tue, 20 Oct 2020 08:35:47 -0400 X-MC-Unique: 7tVmVpDnOtmNsPu2NN0UrQ-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id ED7C61006C93; Tue, 20 Oct 2020 12:35:45 +0000 (UTC) Received: from oldenburg2.str.redhat.com (ovpn-112-119.ams2.redhat.com [10.36.112.119]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E24D06266E; Tue, 20 Oct 2020 12:35:44 +0000 (UTC) From: Florian Weimer To: Adhemerval Zanella Cc: Adhemerval Zanella via Libc-alpha , James Clarke , John Paul Adrian Glaubitz Subject: Re: [PATCH v2 4/9] linux: Use getdents64 on non-LFS readdir References: <20201002170620.1611673-1-adhemerval.zanella@linaro.org> <20201002170620.1611673-5-adhemerval.zanella@linaro.org> <87d01mrub9.fsf@oldenburg2.str.redhat.com> <944cda4b-0681-0000-162d-80cc2b805111@linaro.org> <878sc2d3wt.fsf@oldenburg2.str.redhat.com> <902d1573-3a8f-dbad-9e3f-78f98744d362@linaro.org> <87blgy6iu2.fsf@oldenburg2.str.redhat.com> <1deadc8a-565a-14c5-92ab-7e206100b7ce@linaro.org> <87o8kx4aau.fsf@oldenburg2.str.redhat.com> <8459ff9c-1d1a-d8e4-69c7-02d99f3ae17e@linaro.org> Date: Tue, 20 Oct 2020 14:35:42 +0200 In-Reply-To: <8459ff9c-1d1a-d8e4-69c7-02d99f3ae17e@linaro.org> (Adhemerval Zanella's message of "Tue, 20 Oct 2020 09:05:49 -0300") Message-ID: <878sc12hy9.fsf@oldenburg2.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-6.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Oct 2020 12:35:51 -0000 * Adhemerval Zanella: > On 20/10/2020 04:38, Florian Weimer wrote: >> * Adhemerval Zanella via Libc-alpha: >> >>> ENAMETOOLONG does make more sense, I have fixed it locally. I am not >>> sure I understood by 'delayed', it it report on the readdir call, but >>> the next entry will still be reported in the next readdir call. The >>> construction such as won't work: >>> >>> while (readdir (dp) != NULL) >>> { >>> [...] >>> } >>> >>> But this will work as intended: >>> >>> while (1) >>> { >>> errno = 0; >>> struct dirent *entry = readdir (dp); >>> if (entry == NULL) >>> { >>> if (errno == ENAMETOOLONG) >>> continue; >>> break; >>> } >>> } >> >> I think it's unreasonable to expect that this pattern will work. Itg >> assumes that readdir updates dp despite the error. In other >> implementations, this may produce an endless loop. This is why for >> readdir_r, we record the ENAMETOOLONG error as pending, and report it >> only at the end. (We should do the same for EOVERFLOW errors.) > > This is what POSIX states as the way to check for errors: > > "Applications wishing to check for error situations should set errno > to 0 before calling readdir(). If errno is set to non-zero on return, > an error occurred." > > What the standard is not clear is whether the position as the directory > stream is updated in a case of failure. Yes, that was my concern. I found this in POSIX (in 2.3 Error Numbers): | If an error condition is detected, the action requested may have been | partially performed, unless otherwise stated. But in general we would be rather concerned if a regular read or write on a file descriptor failed *and* still updated the file offset. Not sure about directory streams. > I think it is a fair assumption > since it also states that 'When the end of the directory is encountered, > a null pointer shall be returned and errno is not changed.'. So there > is a distinction between end of directory stream and a failure (in the > example above checking for 'errno == 0' should indicate the end of the > stream). The question is whether you are supposed to be able to continue reading and encounter errno == 0 eventually. In general, this is simply not true, consider EIO or EUCLEAN. This is why I went with delayed error reporting for readdir_r: it is simply not reasonable to expect that applications continue calling readddir_r or readdir after the first error because it's not clear how to can avoid an endless loop (even assuming that the failing readdir call somehow advanced the read pointer). I hope this explains my point of view. > And it also defines EOVERFLOW as possible transient error. It > is possible to do for readdir_r because the function return is different > than the returned 'direct' itself, for readdir what we have is the > 'errno' to signal such failure. I don't understand this point. The approach to detect EOF vs error is different for readdir_r, but the question whether continue after the first error is equally relevant. >>> Even if go to buffer resize, application will still need to handle >>> ENOMEM so I am not sure if using separated buffer is really an >>> improvement here (at least on trying to adequate usual way to call >>> opendir functions). >> >> ENOMEM is a temporary error, ENAMETOOLONG depends on file system content >> and may be much more difficult to resolve. > > Even with ENOMEM, application will require to actually handle a possible > readdir failure and acts accordingly (either by retrying or skip the > file). But I consider failing with ENOMEM in fact much more confusing, > it exposes internal implementation that needs to be handled by the > application (the internal memory reallocation), where ENAMETOOLONG is > indeed a system limitation that can be resolved by using LFS support. I think readdir can already fail with ENOMEM if the error comes from the kernel, for example from ext4_ind_map_blocks and ext4_alloc_branch. So adding another corner case where ENOMEM can result does not seem egregiously bad to me. Thanks, Florian -- Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn, Commercial register: Amtsgericht Muenchen, HRB 153243, Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill