From unknown Wed Jun 18 00:30:03 2025 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-Mailer: MIME-tools 5.509 (Entity 5.509) Content-Type: text/plain; charset=utf-8 From: bug#11816 <11816@debbugs.gnu.org> To: bug#11816 <11816@debbugs.gnu.org> Subject: Status: sort -o: error comes late if opening the outfile fails Reply-To: bug#11816 <11816@debbugs.gnu.org> Date: Wed, 18 Jun 2025 07:30:03 +0000 retitle 11816 sort -o: error comes late if opening the outfile fails reassign 11816 coreutils submitter 11816 Bernhard Voelker severity 11816 normal thanks From debbugs-submit-bounces@debbugs.gnu.org Fri Jun 29 07:59:14 2012 Received: (at submit) by debbugs.gnu.org; 29 Jun 2012 11:59:14 +0000 Received: from localhost ([127.0.0.1]:36628 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SkZqz-0002bq-Ho for submit@debbugs.gnu.org; Fri, 29 Jun 2012 07:59:14 -0400 Received: from eggs.gnu.org ([208.118.235.92]:58709) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SkZqx-0002bj-I3 for submit@debbugs.gnu.org; Fri, 29 Jun 2012 07:59:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SkZmi-0003pY-AE for submit@debbugs.gnu.org; Fri, 29 Jun 2012 07:54:53 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.2 Received: from lists.gnu.org ([208.118.235.17]:51766) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SkZmi-0003pS-6g for submit@debbugs.gnu.org; Fri, 29 Jun 2012 07:54:48 -0400 Received: from eggs.gnu.org ([208.118.235.92]:53346) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SkZmd-0004iG-Iz for bug-coreutils@gnu.org; Fri, 29 Jun 2012 07:54:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SkZmW-0003nD-TJ for bug-coreutils@gnu.org; Fri, 29 Jun 2012 07:54:42 -0400 Received: from moutng.kundenserver.de ([212.227.126.187]:55786) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SkZmW-0003mw-Jm for bug-coreutils@gnu.org; Fri, 29 Jun 2012 07:54:36 -0400 Received: from [192.168.2.108] (p4FF744EA.dip.t-dialin.net [79.247.68.234]) by mrelayeu.kundenserver.de (node=mrbap0) with ESMTP (Nemesis) id 0MN6y0-1SmldJ46TQ-006d57; Fri, 29 Jun 2012 13:54:34 +0200 Message-ID: <4FED9778.20001@bernhard-voelker.de> Date: Fri, 29 Jun 2012 13:54:32 +0200 From: Bernhard Voelker User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120601 Thunderbird/13.0 MIME-Version: 1.0 To: bug-coreutils@gnu.org Subject: sort -o: error comes late if opening the outfile fails Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-Provags-ID: V02:K0:5C8b87dFo1sE9FtCtOHCsAxVaKY2YJe2S89NmiSsDFk wIkeqc4UCet+e0DdDYYjZ6knOJzsZVydE26eutZPQBN0ppVw1J HnjMoYislFwqRApb4he0j6xS+zfxyQFiT/Jb28tULb5kjBmsyF BvK4F/UQVsVoGiF0uiS+smbZrPfpAqwBKbCoqlfhasZEjBNqsU TJ2l/b5BdBJIYC4SjKO8ZBcVxLhluz+euWbPU51TpV/8QIYMXJ PuH37eIr/+55MyR3533bptnzVQ5UToRvs3MLxXaKczaIQQlItr QtF3YCTvMfLUjXGYrC1u1ZiVym0c2ES/d60OKTG8DXlyr3tp5o QWddP0jBwTwFIUEgCHKP/aahZ1bUH9G5cvYLgGNVz X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 208.118.235.17 X-Spam-Score: -6.9 (------) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.9 (------) If opening the output file for writing is not possible - e.g. because the user doesn't have sufficient privileges, then sort issues an error. The problem is that the whole - then useless - computation is already done. In the following little example, it took ~15s to sort the data, and then to realize that it can't write the result: $ time seq 1000000 | src/sort --random-sort -o /cantwritehere src/sort: open failed: /cantwritehere: Permission denied real 0m14.955s user 0m14.936s sys 0m0.042s I'd have expected sort to give the error immediately after startup instead of wasting time for sorting. Shouldn't sort open the outfile right at the beginning (unless in==out), or is this behavior required by some standard? Have a nice day, Berny From debbugs-submit-bounces@debbugs.gnu.org Fri Jun 29 09:00:27 2012 Received: (at 11816) by debbugs.gnu.org; 29 Jun 2012 13:00:27 +0000 Received: from localhost ([127.0.0.1]:36717 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SkaoD-00041o-Kh for submit@debbugs.gnu.org; Fri, 29 Jun 2012 09:00:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:6383) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SkaoB-00041e-KA for 11816@debbugs.gnu.org; Fri, 29 Jun 2012 09:00:24 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q5TCu2Er032757 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 29 Jun 2012 08:56:02 -0400 Received: from [10.3.113.83] (ovpn-113-83.phx2.redhat.com [10.3.113.83]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q5TCu0M3001242 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Fri, 29 Jun 2012 08:56:01 -0400 Message-ID: <4FEDA5DF.6000401@draigBrady.com> Date: Fri, 29 Jun 2012 13:55:59 +0100 From: =?ISO-8859-1?Q?P=E1draig_Brady?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:6.0) Gecko/20110816 Thunderbird/6.0 MIME-Version: 1.0 To: Bernhard Voelker Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails References: <4FED9778.20001@bernhard-voelker.de> In-Reply-To: <4FED9778.20001@bernhard-voelker.de> X-Enigmail-Version: 1.3.2 Content-Type: text/plain; charset=ISO-8859-1 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 Content-Transfer-Encoding: quoted-printable X-MIME-Autoconverted: from 8bit to quoted-printable by mx1.redhat.com id q5TCu2Er032757 X-Spam-Score: -6.9 (------) X-Debbugs-Envelope-To: 11816 Cc: 11816@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.9 (------) On 06/29/2012 12:54 PM, Bernhard Voelker wrote: > If opening the output file for writing is not possible - e.g. because t= he user > doesn't have sufficient privileges, then sort issues an error. The prob= lem > is that the whole - then useless - computation is already done. >=20 > In the following little example, it took ~15s to sort the data, > and then to realize that it can't write the result: >=20 > $ time seq 1000000 | src/sort --random-sort -o /cantwritehere > src/sort: open failed: /cantwritehere: Permission denied >=20 > real 0m14.955s > user 0m14.936s > sys 0m0.042s >=20 > I'd have expected sort to give the error immediately after startup > instead of wasting time for sorting. > Shouldn't sort open the outfile right at the beginning (unless in=3D=3D= out), > or is this behavior required by some standard? Hmm I think that would be an improvement. Now if there was an issue with sorting (like running out of resources), we'd prefer not to leave the empty output hanging around. Also the in=3D=3Dout case, you'd like to check for write-ability too. Both cases could be handled I think with something like: if (access (outfile, W_OK) !=3D 0 && errno !=3D ENOENT) error (...); cheers, P=E1draig. From debbugs-submit-bounces@debbugs.gnu.org Sat Jun 30 07:57:35 2012 Received: (at 11816) by debbugs.gnu.org; 30 Jun 2012 11:57:35 +0000 Received: from localhost ([127.0.0.1]:38727 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SkwIx-0000TS-Hm for submit@debbugs.gnu.org; Sat, 30 Jun 2012 07:57:35 -0400 Received: from smtp.cs.ucla.edu ([131.179.128.62]:52871) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SkwIu-0000TK-TD for 11816@debbugs.gnu.org; Sat, 30 Jun 2012 07:57:34 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id D0CA2A6000E; Sat, 30 Jun 2012 04:53:08 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 49hfyuoOFhzm; Sat, 30 Jun 2012 04:53:08 -0700 (PDT) Received: from [192.168.1.16] (unknown [68.171.84.68]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 10582A60004; Sat, 30 Jun 2012 04:53:07 -0700 (PDT) Message-ID: <4FEEE8A2.2020204@cs.ucla.edu> Date: Sat, 30 Jun 2012 06:53:06 -0500 From: Paul Eggert User-Agent: Mozilla/5.0 (X11; Linux i686; rv:13.0) Gecko/20120615 Thunderbird/13.0.1 MIME-Version: 1.0 To: =?ISO-8859-1?Q?P=E1draig_Brady?= Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails References: <4FED9778.20001@bernhard-voelker.de> <4FEDA5DF.6000401@draigBrady.com> In-Reply-To: <4FEDA5DF.6000401@draigBrady.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -1.9 (-) X-Debbugs-Envelope-To: 11816 Cc: 11816@debbugs.gnu.org, Bernhard Voelker X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -1.9 (-) On 06/29/2012 07:55 AM, P=E1draig Brady wrote: > Also the in=3D=3Dout case, you'd like to check for write-ability too. > > Both cases could be handled I think with something like: > > if (access (outfile, W_OK) !=3D 0 && errno !=3D ENOENT) > error (...); Wouldn't it be better to actually open the output file, but not truncate it? We can then truncate it just before actually writing to the file. That would avoid a race condition or two. In the in=3D=3Dout case, we could tune this by opening the file just once, with O_RDWR. If the file is not a regular file, we might have to give up and open such a file twice, but that should be rare. From debbugs-submit-bounces@debbugs.gnu.org Sat Jun 30 09:15:56 2012 Received: (at 11816) by debbugs.gnu.org; 30 Jun 2012 13:15:56 +0000 Received: from localhost ([127.0.0.1]:38851 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SkxWl-0002FC-EQ for submit@debbugs.gnu.org; Sat, 30 Jun 2012 09:15:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:29211) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SkxWi-0002F1-Rq for 11816@debbugs.gnu.org; Sat, 30 Jun 2012 09:15:53 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q5UDBArg021819 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Sat, 30 Jun 2012 09:11:11 -0400 Received: from [10.3.113.31] (ovpn-113-31.phx2.redhat.com [10.3.113.31]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q5UDB89X011200 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Sat, 30 Jun 2012 09:11:09 -0400 Message-ID: <4FEEFAEC.2020000@draigBrady.com> Date: Sat, 30 Jun 2012 14:11:08 +0100 From: =?ISO-8859-1?Q?P=E1draig_Brady?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:6.0) Gecko/20110816 Thunderbird/6.0 MIME-Version: 1.0 To: Paul Eggert Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails References: <4FED9778.20001@bernhard-voelker.de> <4FEDA5DF.6000401@draigBrady.com> <4FEEE8A2.2020204@cs.ucla.edu> In-Reply-To: <4FEEE8A2.2020204@cs.ucla.edu> X-Enigmail-Version: 1.3.2 Content-Type: text/plain; charset=ISO-8859-1 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 Content-Transfer-Encoding: quoted-printable X-MIME-Autoconverted: from 8bit to quoted-printable by mx1.redhat.com id q5UDBArg021819 X-Spam-Score: -6.9 (------) X-Debbugs-Envelope-To: 11816 Cc: 11816@debbugs.gnu.org, Bernhard Voelker X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.9 (------) On 06/30/2012 12:53 PM, Paul Eggert wrote: > On 06/29/2012 07:55 AM, P=E1draig Brady wrote: >> Also the in=3D=3Dout case, you'd like to check for write-ability too. >> >> Both cases could be handled I think with something like: >> >> if (access (outfile, W_OK) !=3D 0 && errno !=3D ENOENT) >> error (...); >=20 > Wouldn't it be better to actually open the output file, > but not truncate it? We can then truncate it just before > actually writing to the file. That would avoid a race > condition or two. >=20 > In the in=3D=3Dout case, we could tune this by opening > the file just once, with O_RDWR. If the file is not > a regular file, we might have to give up and open such > a file twice, but that should be rare. >=20 The race would be unlikely and only fallback to the existing operation of slower failure. Though I suppose opening the file is a more direct check and would also obviate the need to check for writeability of the containing dir in the case of a non existent file. OK I'm leaning towards an early open so. As for cleaning up an empty created file, `sort` already has an exit_cleanup() function, so we can unlink there. I'm not sure it's worth tuning the in=3D=3Dout case TBH. cheers, P=E1draig. From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 02 06:26:36 2012 Received: (at 11816-done) by debbugs.gnu.org; 2 Jul 2012 10:26:36 +0000 Received: from localhost ([127.0.0.1]:41282 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1Sldpz-00065b-BG for submit@debbugs.gnu.org; Mon, 02 Jul 2012 06:26:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:14532) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1Sldpv-00065S-S1 for 11816-done@debbugs.gnu.org; Mon, 02 Jul 2012 06:26:33 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q62ALkdJ005434 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 2 Jul 2012 06:21:47 -0400 Received: from [10.3.113.87] (ovpn-113-87.phx2.redhat.com [10.3.113.87]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q62ALfrL012669 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Mon, 2 Jul 2012 06:21:44 -0400 Message-ID: <4FF17632.4000601@draigBrady.com> Date: Mon, 02 Jul 2012 12:21:38 +0200 From: =?ISO-8859-1?Q?P=E1draig_Brady?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:6.0) Gecko/20110816 Thunderbird/6.0 MIME-Version: 1.0 To: Paul Eggert Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails References: <4FED9778.20001@bernhard-voelker.de> <4FEDA5DF.6000401@draigBrady.com> <4FEEE8A2.2020204@cs.ucla.edu> <4FEEFAEC.2020000@draigBrady.com> In-Reply-To: <4FEEFAEC.2020000@draigBrady.com> X-Enigmail-Version: 1.3.2 Content-Type: multipart/mixed; boundary="------------080300090800090106060105" X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Spam-Score: -6.9 (------) X-Debbugs-Envelope-To: 11816-done Cc: 11816-done@debbugs.gnu.org, Bernhard Voelker X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.9 (------) This is a multi-part message in MIME format. --------------080300090800090106060105 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-MIME-Autoconverted: from 8bit to quoted-printable by mx1.redhat.com id q62ALkdJ005434 On 06/30/2012 03:11 PM, P=E1draig Brady wrote: > On 06/30/2012 12:53 PM, Paul Eggert wrote: >> On 06/29/2012 07:55 AM, P=E1draig Brady wrote: >>> Also the in=3D=3Dout case, you'd like to check for write-ability too. >>> >>> Both cases could be handled I think with something like: >>> >>> if (access (outfile, W_OK) !=3D 0 && errno !=3D ENOENT) >>> error (...); >> >> Wouldn't it be better to actually open the output file, >> but not truncate it? We can then truncate it just before >> actually writing to the file. That would avoid a race >> condition or two. >> >> In the in=3D=3Dout case, we could tune this by opening >> the file just once, with O_RDWR. If the file is not >> a regular file, we might have to give up and open such >> a file twice, but that should be rare. >> >=20 > The race would be unlikely and > only fallback to the existing operation > of slower failure. >=20 > Though I suppose opening the file is a > more direct check and would also obviate the > need to check for writeability of the containing dir > in the case of a non existent file. >=20 > OK I'm leaning towards an early open so. >=20 > As for cleaning up an empty created file, > `sort` already has an exit_cleanup() function, > so we can unlink there. >=20 > I'm not sure it's worth tuning the in=3D=3Dout case TBH. So I didn't bother unlinking created empty files as this is problematic in the presence of symlinks. To mitigate this I create the output after all option validation is done, just before sort/merge process is started. Also we must be careful to handle the `sort -o missing missing` case. I.E. we don't want to create an empty file, resulting in the above failing to notice the missing file and returning succesfully. So to avoid that I explicitly check all inputs are readable first. In addition to catering for the above case, it's a general improvement to avoid redundant processing. That was already handled in the merge cas= e, but in the sorting case only a stat was done as a side effect of input size checking, and that didn't handle the case where input was present but unreadable. Patch attached. cheers, P=E1draig. --------------080300090800090106060105 Content-Type: text/plain; name="sort-exit-early.diff" Content-Disposition: attachment; filename="sort-exit-early.diff" Content-Transfer-Encoding: 7bit >From f5601f0e6d0be43a8976839482abe4726f75b4a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= Date: Sun, 1 Jul 2012 01:14:42 +0100 Subject: [PATCH] sort: avoid redundant processing with inaccessible inputs or output * src/sort.c (check_inputs): A new function to verify all inputs are accessible before further processing. (check_output): A new function to open or create a specified output file, before futher processing. (stream_open): Adjust to truncating the previously opened output file rather than opening directly. (avoid_trashing_input): Optimize to stat the output file descriptor, rather than the file name. (main): Call the new functions to check accessability of inputs and output, before processing starts. * tests/misc/sort: Adjust to the changed error message. * tests/misc/sort-merge-fdlimit: Account for the earlier opened file descriptor of the specified output file. * tests/misc/sort-exit-early: A new test to exercise the improvements. * tests/Makefile.am: Reference the new test. * NEWS: Mention the improvement. Suggested-by: Bernhard Voelker --- NEWS | 6 ++++ src/sort.c | 65 ++++++++++++++++++++++++++++++++++++++-- tests/Makefile.am | 1 + tests/misc/sort | 2 +- tests/misc/sort-exit-early | 34 +++++++++++++++++++++ tests/misc/sort-merge-fdlimit | 2 +- 6 files changed, 104 insertions(+), 6 deletions(-) create mode 100755 tests/misc/sort-exit-early diff --git a/NEWS b/NEWS index 8c75a32..c51fb78 100644 --- a/NEWS +++ b/NEWS @@ -41,6 +41,12 @@ GNU coreutils NEWS -*- outline -*- patches as well as enough support to build on the Hurd, we no longer have any reason to include it here. +** Improvements + + sort will avoid redundant processing in the presence of inaccessible inputs, + or unwritable output. Immediate errors are now given before any potentially + expensive processing is initiated. + * Noteworthy changes in release 8.17 (2012-05-10) [stable] diff --git a/src/sort.c b/src/sort.c index 5a48ce6..40f094b 100644 --- a/src/sort.c +++ b/src/sort.c @@ -357,6 +357,9 @@ static bool unique; /* Nonzero if any of the input files are the standard input. */ static bool have_read_stdin; +/* File descriptor associated with -o. */ +static int outfd = -1; + /* List of key field comparisons to be tried. */ static struct keyfield *keylist; @@ -911,11 +914,12 @@ create_temp_file (int *pfd, bool survive_fd_exhaustion) static FILE * stream_open (char const *file, char const *how) { + FILE *fp; + if (!file) return stdout; if (*how == 'r') { - FILE *fp; if (STREQ (file, "-")) { have_read_stdin = true; @@ -924,9 +928,18 @@ stream_open (char const *file, char const *how) else fp = fopen (file, how); fadvise (fp, FADVISE_SEQUENTIAL); - return fp; } - return fopen (file, how); + else if (*how == 'w') + { + assert (outfd != -1); + if (ftruncate (outfd, 0) != 0) + error (EXIT_FAILURE, errno, _("%s: error truncating"), quote (file)); + fp = fdopen (outfd, how); + } + else + assert (!"unexpected mode passed to stream_open"); + + return fp; } /* Same as stream_open, except always return a non-null value; die on @@ -3637,7 +3650,7 @@ avoid_trashing_input (struct sortfile *files, size_t ntemps, if (! got_outstat) { if ((outfile - ? stat (outfile, &outstat) + ? fstat (outfd, &outstat) : fstat (STDOUT_FILENO, &outstat)) != 0) break; @@ -3666,6 +3679,44 @@ avoid_trashing_input (struct sortfile *files, size_t ntemps, } } +/* Scan the input files to ensure all are accessible. + Otherwise exit with a diagnostic. + + Note this will catch common issues with permissions etc. + but will fail to notice issues where you can open() but not read(), + like when a directory is specified on some systems. + Catching these obscure cases could slow down performance in + common cases. */ + +static void +check_inputs (char * const *files, size_t nfiles) +{ + size_t i; + for (i = 0; i < nfiles; i++) + { + if (STREQ (files[i], "-")) + continue; + + if (euidaccess (files[i], R_OK) != 0) + die (_("cannot read"), files[i]); + } +} + +/* Ensure a specified output file can be created or written to, + and cache the returned descriptor in the global OUTFD variable. + Otherwise exit with a diagnostic. */ + +static void +check_output (char const *outfile) +{ + if (outfile) + { + outfd = open (outfile, O_WRONLY | O_CREAT | O_BINARY, MODE_RW_UGO); + if (outfd < 0) + die (_("open failed"), outfile); + } +} + /* Merge the input FILES. NTEMPS is the number of files at the start of FILES that are temporary; it is zero at the top level. NFILES is the total number of files. Put the output in @@ -4620,6 +4671,12 @@ main (int argc, char **argv) exit (check (files[0], checkonly) ? EXIT_SUCCESS : SORT_OUT_OF_ORDER); } + /* Check all inputs are accessible, or exit immediately. */ + check_inputs (files, nfiles); + + /* Check output is writable, or exit immediately. */ + check_output (outfile); + if (mergeonly) { struct sortfile *sortfiles = xcalloc (nfiles, sizeof *sortfiles); diff --git a/tests/Makefile.am b/tests/Makefile.am index 2155cee..dfac9a0 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -251,6 +251,7 @@ TESTS = \ misc/sort-merge \ misc/sort-merge-fdlimit \ misc/sort-month \ + misc/sort-exit-early \ misc/sort-rand \ misc/sort-spinlock-abuse \ misc/sort-stale-thread-mem \ diff --git a/tests/misc/sort b/tests/misc/sort index 5be00a0..5d15d75 100755 --- a/tests/misc/sort +++ b/tests/misc/sort @@ -33,7 +33,7 @@ my $mb_locale = $ENV{LOCALE_FR_UTF8}; # Normalize each diagnostic to use '-'. my $normalize_filename = {ERR_SUBST => 's/^$prog: .*?:/$prog: -:/'}; -my $no_file = "$prog: open failed: no-file: No such file or directory\n"; +my $no_file = "$prog: cannot read: no-file: No such file or directory\n"; my @Tests = ( diff --git a/tests/misc/sort-exit-early b/tests/misc/sort-exit-early new file mode 100755 index 0000000..4cdcee6 --- /dev/null +++ b/tests/misc/sort-exit-early @@ -0,0 +1,34 @@ +#!/bin/sh +# Test 'sort' exits early on inaccessible inputs or output + +# Copyright (C) 2012 Free Software Foundation, Inc. + +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +. "${srcdir=.}/init.sh"; path_prepend_ ../src +print_ver_ sort + + +timeout 10 sort -o . && fail=1 +test $? == 124 && fail=1 + +# Check all inputs are readable before starting to sort +# Also ensure the output isn't created in this case +touch output +chmod a-r output +timeout 10 sort -o typo - output && fail=1 +test $? == 124 && fail=1 +test -e typo && fail=1 + +Exit $fail diff --git a/tests/misc/sort-merge-fdlimit b/tests/misc/sort-merge-fdlimit index 7ce109c..ee1575a 100755 --- a/tests/misc/sort-merge-fdlimit +++ b/tests/misc/sort-merge-fdlimit @@ -65,7 +65,7 @@ done (seq 6 && echo 6) >exp || fail=1 echo 6 >out || fail=1 (exec 3<&- 4<&- 5<&- 6) id 1Sleqj-0008OP-E6 for submit@debbugs.gnu.org; Mon, 02 Jul 2012 07:31:26 -0400 Received: from mx.meyering.net ([88.168.87.75]:59432) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1Sleqg-0008OH-TW for 11816@debbugs.gnu.org; Mon, 02 Jul 2012 07:31:23 -0400 Received: from rho.meyering.net (rho.meyering.net [127.0.0.1]) by rho.meyering.net (Acme Bit-Twister) with ESMTP id 7EEDF60118; Mon, 2 Jul 2012 13:26:46 +0200 (CEST) From: Jim Meyering To: 11816@debbugs.gnu.org Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails In-Reply-To: <4FF17632.4000601@draigBrady.com> (=?iso-8859-1?Q?=22P=E1drai?= =?iso-8859-1?Q?g?= Brady"'s message of "Mon, 02 Jul 2012 12:21:38 +0200") References: <4FED9778.20001@bernhard-voelker.de> <4FEDA5DF.6000401@draigBrady.com> <4FEEE8A2.2020204@cs.ucla.edu> <4FEEFAEC.2020000@draigBrady.com> <4FF17632.4000601@draigBrady.com> Date: Mon, 02 Jul 2012 13:26:46 +0200 Message-ID: <877gum9zex.fsf@rho.meyering.net> Lines: 45 MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -1.9 (-) X-Debbugs-Envelope-To: 11816 Cc: P@draigBrady.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -1.9 (-) P=E1draig Brady wrote: ... > Subject: [PATCH] sort: avoid redundant processing with inaccessible input= s or > output > > * src/sort.c (check_inputs): A new function to verify all inputs > are accessible before further processing. > (check_output): A new function to open or create a specified > output file, before futher processing. > (stream_open): Adjust to truncating the previously opened > output file rather than opening directly. > (avoid_trashing_input): Optimize to stat the output file > descriptor, rather than the file name. > (main): Call the new functions to check accessability of Hi P=E1draig, Thanks for dealing with this. So far, I've read through the commit log and NEWS: s/accessability/accessibility/ > inputs and output, before processing starts. ... > diff --git a/NEWS b/NEWS > index 8c75a32..c51fb78 100644 > --- a/NEWS > +++ b/NEWS > @@ -41,6 +41,12 @@ GNU coreutils NEWS = -*- outline -*- > patches as well as enough support to build on the Hurd, we no longer > have any reason to include it here. > > +** Improvements > + > + sort will avoid redundant processing in the presence of inaccessible i= nputs, s/will avoid/avoids/ (or "now avoids") ? > + or unwritable output. Immediate errors are now given before any poten= tially > + expensive processing is initiated. Maybe replace that "Immediate ..." sentence with something like this: Sort now diagnoses certain errors at start-up, rather than only after potentially expensive processing. From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 02 13:00:14 2012 Received: (at 11816) by debbugs.gnu.org; 2 Jul 2012 17:00:14 +0000 Received: from localhost ([127.0.0.1]:42915 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1Sljyv-0000uK-1K for submit@debbugs.gnu.org; Mon, 02 Jul 2012 13:00:14 -0400 Received: from mx.meyering.net ([88.168.87.75]:60365) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1Sljyt-0000uE-3p for 11816@debbugs.gnu.org; Mon, 02 Jul 2012 13:00:11 -0400 Received: from rho.meyering.net (rho.meyering.net [127.0.0.1]) by rho.meyering.net (Acme Bit-Twister) with ESMTP id 8B12A60177; Mon, 2 Jul 2012 18:55:32 +0200 (CEST) From: Jim Meyering To: 11816@debbugs.gnu.org Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails In-Reply-To: <4FF17632.4000601@draigBrady.com> (=?iso-8859-1?Q?=22P=E1drai?= =?iso-8859-1?Q?g?= Brady"'s message of "Mon, 02 Jul 2012 12:21:38 +0200") References: <4FED9778.20001@bernhard-voelker.de> <4FEDA5DF.6000401@draigBrady.com> <4FEEE8A2.2020204@cs.ucla.edu> <4FEEFAEC.2020000@draigBrady.com> <4FF17632.4000601@draigBrady.com> Date: Mon, 02 Jul 2012 18:55:32 +0200 Message-ID: <87d34e85mj.fsf@rho.meyering.net> Lines: 29 MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -1.9 (-) X-Debbugs-Envelope-To: 11816 Cc: P@draigBrady.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -1.9 (-) P=E1draig Brady wrote: >... > diff --git a/tests/misc/sort-exit-early b/tests/misc/sort-exit-early ... > +test $? =3D=3D 124 && fail=3D1 > + > +# Check all inputs are readable before starting to sort > +# Also ensure the output isn't created in this case > +touch output > +chmod a-r output > +timeout 10 sort -o typo - output && fail=3D1 > +test $? =3D=3D 124 && fail=3D1 "make syntax-check" dinged these: tests/misc/sort-exit-early:24:test $? =3D=3D 124 && fail=3D1 tests/misc/sort-exit-early:31:test $? =3D=3D 124 && fail=3D1 maint.mk: use "test x =3D x", not "test x =3D=3D x" make: *** [sc_prohibit_test_double_equal] Error 1 Here's a fix for a formatting nit in the sources: static void -check_inputs (char * const *files, size_t nfiles) +check_inputs (char *const *files, size_t nfiles) Otherwise, it all looks fine. Thanks again. From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 02 15:29:35 2012 Received: (at 11816) by debbugs.gnu.org; 2 Jul 2012 19:29:35 +0000 Received: from localhost ([127.0.0.1]:43260 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SlmJT-0005zh-JV for submit@debbugs.gnu.org; Mon, 02 Jul 2012 15:29:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:64802) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SlmJN-0005zU-Hu for 11816@debbugs.gnu.org; Mon, 02 Jul 2012 15:29:34 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q62JOqDT031477 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 2 Jul 2012 15:24:52 -0400 Received: from [10.3.113.81] (ovpn-113-81.phx2.redhat.com [10.3.113.81]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q62JOnb5016116 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Mon, 2 Jul 2012 15:24:51 -0400 Message-ID: <4FF1F580.4010909@draigBrady.com> Date: Mon, 02 Jul 2012 21:24:48 +0200 From: =?ISO-8859-1?Q?P=E1draig_Brady?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:6.0) Gecko/20110816 Thunderbird/6.0 MIME-Version: 1.0 To: Jim Meyering Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails References: <4FED9778.20001@bernhard-voelker.de> <4FEDA5DF.6000401@draigBrady.com> <4FEEE8A2.2020204@cs.ucla.edu> <4FEEFAEC.2020000@draigBrady.com> <4FF17632.4000601@draigBrady.com> <87d34e85mj.fsf@rho.meyering.net> In-Reply-To: <87d34e85mj.fsf@rho.meyering.net> X-Enigmail-Version: 1.3.2 Content-Type: text/plain; charset=ISO-8859-1 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.25 Content-Transfer-Encoding: quoted-printable X-MIME-Autoconverted: from 8bit to quoted-printable by mx1.redhat.com id q62JOqDT031477 X-Spam-Score: -6.9 (------) X-Debbugs-Envelope-To: 11816 Cc: 11816@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.9 (------) On 07/02/2012 06:55 PM, Jim Meyering wrote: > P=E1draig Brady wrote: >> ... >> diff --git a/tests/misc/sort-exit-early b/tests/misc/sort-exit-early > ... >> +test $? =3D=3D 124 && fail=3D1 >> + >> +# Check all inputs are readable before starting to sort >> +# Also ensure the output isn't created in this case >> +touch output >> +chmod a-r output >> +timeout 10 sort -o typo - output && fail=3D1 >> +test $? =3D=3D 124 && fail=3D1 >=20 > "make syntax-check" dinged these: >=20 > tests/misc/sort-exit-early:24:test $? =3D=3D 124 && fail=3D1 > tests/misc/sort-exit-early:31:test $? =3D=3D 124 && fail=3D1 > maint.mk: use "test x =3D x", not "test x =3D=3D x" > make: *** [sc_prohibit_test_double_equal] Error 1 >=20 >=20 > Here's a fix for a formatting nit in the sources: >=20 > static void > -check_inputs (char * const *files, size_t nfiles) > +check_inputs (char *const *files, size_t nfiles) >=20 > Otherwise, it all looks fine. > Thanks again. Pushed. thanks for the careful review. P=E1draig. From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 02 18:54:27 2012 Received: (at 11816) by debbugs.gnu.org; 2 Jul 2012 22:54:27 +0000 Received: from localhost ([127.0.0.1]:43595 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SlpVi-0002C7-Td for submit@debbugs.gnu.org; Mon, 02 Jul 2012 18:54:27 -0400 Received: from smtp.cs.ucla.edu ([131.179.128.62]:44771) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SlpVg-0002By-6i for 11816@debbugs.gnu.org; Mon, 02 Jul 2012 18:54:25 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id 8D5F0A60006; Mon, 2 Jul 2012 15:49:45 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 8sNbIEuDPqCu; Mon, 2 Jul 2012 15:49:45 -0700 (PDT) Received: from [192.168.1.4] (pool-108-23-119-2.lsanca.fios.verizon.net [108.23.119.2]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id D1011A60005; Mon, 2 Jul 2012 15:49:44 -0700 (PDT) Message-ID: <4FF22589.2000401@cs.ucla.edu> Date: Mon, 02 Jul 2012 15:49:45 -0700 From: Paul Eggert Organization: UCLA Computer Science Department User-Agent: Mozilla/5.0 (X11; Linux i686; rv:13.0) Gecko/20120615 Thunderbird/13.0.1 MIME-Version: 1.0 To: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails References: <4FED9778.20001@bernhard-voelker.de> <4FEDA5DF.6000401@draigBrady.com> <4FEEE8A2.2020204@cs.ucla.edu> <4FEEFAEC.2020000@draigBrady.com> <4FF17632.4000601@draigBrady.com> In-Reply-To: <4FF17632.4000601@draigBrady.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Score: -1.9 (-) X-Debbugs-Envelope-To: 11816 Cc: 11816@debbugs.gnu.org, Jim Meyering X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -1.9 (-) Thanks for the improvement. How about the following patch to simplify this a bit? It removes a call to fdopen, among other things. >From 05cc1b416a47330ef296dbeadd2a4b6095fe5c7d Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Mon, 2 Jul 2012 15:47:03 -0700 Subject: [PATCH] sort: simplify -o handling to avoid fdopen, assert * src/sort.c (outfd): Remove. All uses replaced by STDOUT_FILENO. (stream_open): When writing, use stdout rather than fdopen. (move_fd_or_die): Renamed from dup2_or_die, with the added functionality of closing its first argument. All uses changed. (avoid_trashing_input): Special case for !outfile no longer needed. (check_output): Arrange for standard output to go to the file, rather than storing the fd in outfd. --- src/sort.c | 47 ++++++++++++++++++++--------------------------- 1 files changed, 20 insertions(+), 27 deletions(-) diff --git a/src/sort.c b/src/sort.c index e4067c9..f0d32c3 100644 --- a/src/sort.c +++ b/src/sort.c @@ -357,9 +357,6 @@ static bool unique; /* Nonzero if any of the input files are the standard input. */ static bool have_read_stdin; -/* File descriptor associated with -o. */ -static int outfd = -1; - /* List of key field comparisons to be tried. */ static struct keyfield *keylist; @@ -916,8 +913,6 @@ stream_open (char const *file, char const *how) { FILE *fp; - if (!file) - return stdout; if (*how == 'r') { if (STREQ (file, "-")) @@ -931,10 +926,10 @@ stream_open (char const *file, char const *how) } else if (*how == 'w') { - assert (outfd != -1); - if (ftruncate (outfd, 0) != 0) - error (EXIT_FAILURE, errno, _("%s: error truncating"), quote (file)); - fp = fdopen (outfd, how); + if (file && ftruncate (STDOUT_FILENO, 0) != 0) + error (EXIT_FAILURE, errno, _("%s: error truncating"), + quote (file)); + fp = stdout; } else assert (!"unexpected mode passed to stream_open"); @@ -981,10 +976,14 @@ xfclose (FILE *fp, char const *file) } static void -dup2_or_die (int oldfd, int newfd) +move_fd_or_die (int oldfd, int newfd) { - if (dup2 (oldfd, newfd) < 0) - error (SORT_FAILURE, errno, _("dup2 failed")); + if (oldfd != newfd) + { + if (dup2 (oldfd, newfd) < 0) + error (SORT_FAILURE, errno, _("dup2 failed")); + close (oldfd); + } } /* Fork a child process for piping to and do common cleanup. The @@ -1095,10 +1094,8 @@ maybe_create_temp (FILE **pfp, bool survive_fd_exhaustion) else if (node->pid == 0) { close (pipefds[1]); - dup2_or_die (tempfd, STDOUT_FILENO); - close (tempfd); - dup2_or_die (pipefds[0], STDIN_FILENO); - close (pipefds[0]); + move_fd_or_die (tempfd, STDOUT_FILENO); + move_fd_or_die (pipefds[0], STDIN_FILENO); if (execlp (compress_program, compress_program, (char *) NULL) < 0) error (SORT_FAILURE, errno, _("couldn't execute %s"), @@ -1155,10 +1152,8 @@ open_temp (struct tempnode *temp) case 0: close (pipefds[0]); - dup2_or_die (tempfd, STDIN_FILENO); - close (tempfd); - dup2_or_die (pipefds[1], STDOUT_FILENO); - close (pipefds[1]); + move_fd_or_die (tempfd, STDIN_FILENO); + move_fd_or_die (pipefds[1], STDOUT_FILENO); execlp (compress_program, compress_program, "-d", (char *) NULL); error (SORT_FAILURE, errno, _("couldn't execute %s -d"), @@ -3649,10 +3644,7 @@ avoid_trashing_input (struct sortfile *files, size_t ntemps, { if (! got_outstat) { - if ((outfile - ? fstat (outfd, &outstat) - : fstat (STDOUT_FILENO, &outstat)) - != 0) + if (fstat (STDOUT_FILENO, &outstat) != 0) break; got_outstat = true; } @@ -3703,17 +3695,18 @@ check_inputs (char *const *files, size_t nfiles) } /* Ensure a specified output file can be created or written to, - and cache the returned descriptor in the global OUTFD variable. - Otherwise exit with a diagnostic. */ + and point stdout to it. Do not truncate the file. + Exit with a diagnostic on failure. */ static void check_output (char const *outfile) { if (outfile) { - outfd = open (outfile, O_WRONLY | O_CREAT | O_BINARY, MODE_RW_UGO); + int outfd = open (outfile, O_WRONLY | O_CREAT | O_BINARY, MODE_RW_UGO); if (outfd < 0) die (_("open failed"), outfile); + move_fd_or_die (outfd, STDOUT_FILENO); } } -- 1.7.6.5 From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 02 19:12:53 2012 Received: (at 11816) by debbugs.gnu.org; 2 Jul 2012 23:12:53 +0000 Received: from localhost ([127.0.0.1]:43617 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SlpnZ-0003QP-4v for submit@debbugs.gnu.org; Mon, 02 Jul 2012 19:12:53 -0400 Received: from mx1.redhat.com ([209.132.183.28]:1374) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SlpnW-0003QJ-NW for 11816@debbugs.gnu.org; Mon, 02 Jul 2012 19:12:51 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q62N8B9p026737 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 2 Jul 2012 19:08:11 -0400 Received: from [10.3.113.81] (ovpn-113-81.phx2.redhat.com [10.3.113.81]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q62N86EF029352 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Mon, 2 Jul 2012 19:08:08 -0400 Message-ID: <4FF229D4.8030405@draigBrady.com> Date: Tue, 03 Jul 2012 01:08:04 +0200 From: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:6.0) Gecko/20110816 Thunderbird/6.0 MIME-Version: 1.0 To: Paul Eggert Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails References: <4FED9778.20001@bernhard-voelker.de> <4FEDA5DF.6000401@draigBrady.com> <4FEEE8A2.2020204@cs.ucla.edu> <4FEEFAEC.2020000@draigBrady.com> <4FF17632.4000601@draigBrady.com> <4FF22589.2000401@cs.ucla.edu> In-Reply-To: <4FF22589.2000401@cs.ucla.edu> X-Enigmail-Version: 1.3.2 Content-Type: text/plain; charset=UTF-8 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.25 Content-Transfer-Encoding: quoted-printable X-MIME-Autoconverted: from 8bit to quoted-printable by mx1.redhat.com id q62N8B9p026737 X-Spam-Score: -6.9 (------) X-Debbugs-Envelope-To: 11816 Cc: 11816@debbugs.gnu.org, Jim Meyering X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.9 (------) On 07/03/2012 12:49 AM, Paul Eggert wrote: > Thanks for the improvement. > How about the following patch to simplify this a bit? > It removes a call to fdopen, among other things. >=20 >>>From 05cc1b416a47330ef296dbeadd2a4b6095fe5c7d Mon Sep 17 00:00:00 2001 > From: Paul Eggert > Date: Mon, 2 Jul 2012 15:47:03 -0700 > Subject: [PATCH] sort: simplify -o handling to avoid fdopen, assert >=20 > * src/sort.c (outfd): Remove. All uses replaced by STDOUT_FILENO. > (stream_open): When writing, use stdout rather than fdopen. > (move_fd_or_die): Renamed from dup2_or_die, with the added functionalit= y > of closing its first argument. All uses changed. > (avoid_trashing_input): Special case for !outfile no longer needed. > (check_output): Arrange for standard output to go to the file, > rather than storing the fd in outfd. > --- > src/sort.c | 47 ++++++++++++++++++++--------------------------- > 1 files changed, 20 insertions(+), 27 deletions(-) >=20 > diff --git a/src/sort.c b/src/sort.c > index e4067c9..f0d32c3 100644 > --- a/src/sort.c > +++ b/src/sort.c > @@ -357,9 +357,6 @@ static bool unique; > /* Nonzero if any of the input files are the standard input. */ > static bool have_read_stdin; > =20 > -/* File descriptor associated with -o. */ > -static int outfd =3D -1; > - > /* List of key field comparisons to be tried. */ > static struct keyfield *keylist; > =20 > @@ -916,8 +913,6 @@ stream_open (char const *file, char const *how) > { > FILE *fp; > =20 > - if (!file) > - return stdout; > if (*how =3D=3D 'r') > { > if (STREQ (file, "-")) > @@ -931,10 +926,10 @@ stream_open (char const *file, char const *how) > } > else if (*how =3D=3D 'w') > { > - assert (outfd !=3D -1); > - if (ftruncate (outfd, 0) !=3D 0) > - error (EXIT_FAILURE, errno, _("%s: error truncating"), quote (= file)); > - fp =3D fdopen (outfd, how); > + if (file && ftruncate (STDOUT_FILENO, 0) !=3D 0) > + error (EXIT_FAILURE, errno, _("%s: error truncating"), > + quote (file)); Hmm I know I had EXIT_FAILURE here. Should that be SORT_FAILURE? > + fp =3D stdout; > } > else > assert (!"unexpected mode passed to stream_open"); > @@ -981,10 +976,14 @@ xfclose (FILE *fp, char const *file) > } > =20 > static void > -dup2_or_die (int oldfd, int newfd) > +move_fd_or_die (int oldfd, int newfd) > { > - if (dup2 (oldfd, newfd) < 0) > - error (SORT_FAILURE, errno, _("dup2 failed")); > + if (oldfd !=3D newfd) > + { > + if (dup2 (oldfd, newfd) < 0) > + error (SORT_FAILURE, errno, _("dup2 failed")); > + close (oldfd); > + } > } > =20 > /* Fork a child process for piping to and do common cleanup. The > @@ -1095,10 +1094,8 @@ maybe_create_temp (FILE **pfp, bool survive_fd_e= xhaustion) > else if (node->pid =3D=3D 0) > { > close (pipefds[1]); > - dup2_or_die (tempfd, STDOUT_FILENO); > - close (tempfd); > - dup2_or_die (pipefds[0], STDIN_FILENO); > - close (pipefds[0]); > + move_fd_or_die (tempfd, STDOUT_FILENO); > + move_fd_or_die (pipefds[0], STDIN_FILENO); > =20 > if (execlp (compress_program, compress_program, (char *) NUL= L) < 0) > error (SORT_FAILURE, errno, _("couldn't execute %s"), > @@ -1155,10 +1152,8 @@ open_temp (struct tempnode *temp) > =20 > case 0: > close (pipefds[0]); > - dup2_or_die (tempfd, STDIN_FILENO); > - close (tempfd); > - dup2_or_die (pipefds[1], STDOUT_FILENO); > - close (pipefds[1]); > + move_fd_or_die (tempfd, STDIN_FILENO); > + move_fd_or_die (pipefds[1], STDOUT_FILENO); > =20 > execlp (compress_program, compress_program, "-d", (char *) NULL)= ; > error (SORT_FAILURE, errno, _("couldn't execute %s -d"), > @@ -3649,10 +3644,7 @@ avoid_trashing_input (struct sortfile *files, si= ze_t ntemps, > { > if (! got_outstat) > { > - if ((outfile > - ? fstat (outfd, &outstat) > - : fstat (STDOUT_FILENO, &outstat)) > - !=3D 0) > + if (fstat (STDOUT_FILENO, &outstat) !=3D 0) > break; > got_outstat =3D true; > } > @@ -3703,17 +3695,18 @@ check_inputs (char *const *files, size_t nfiles= ) > } > =20 > /* Ensure a specified output file can be created or written to, > - and cache the returned descriptor in the global OUTFD variable. > - Otherwise exit with a diagnostic. */ > + and point stdout to it. Do not truncate the file. > + Exit with a diagnostic on failure. */ > =20 > static void > check_output (char const *outfile) > { > if (outfile) > { > - outfd =3D open (outfile, O_WRONLY | O_CREAT | O_BINARY, MODE_RW_= UGO); > + int outfd =3D open (outfile, O_WRONLY | O_CREAT | O_BINARY, MODE= _RW_UGO); > if (outfd < 0) > die (_("open failed"), outfile); > + move_fd_or_die (outfd, STDOUT_FILENO); > } > } > =20 Looks good! cheers, P=C3=A1draig. From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 02 19:54:21 2012 Received: (at 11816) by debbugs.gnu.org; 2 Jul 2012 23:54:21 +0000 Received: from localhost ([127.0.0.1]:43666 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SlqRh-0004OY-BG for submit@debbugs.gnu.org; Mon, 02 Jul 2012 19:54:21 -0400 Received: from smtp.cs.ucla.edu ([131.179.128.62]:47092) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SlqRf-0004OP-7E for 11816@debbugs.gnu.org; Mon, 02 Jul 2012 19:54:19 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id C6DAC39E800A; Mon, 2 Jul 2012 16:49:40 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 8U5q9xDep34M; Mon, 2 Jul 2012 16:49:40 -0700 (PDT) Received: from [192.168.1.4] (pool-108-23-119-2.lsanca.fios.verizon.net [108.23.119.2]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 018CC39E8008; Mon, 2 Jul 2012 16:49:39 -0700 (PDT) Message-ID: <4FF23395.5090002@cs.ucla.edu> Date: Mon, 02 Jul 2012 16:49:41 -0700 From: Paul Eggert Organization: UCLA Computer Science Department User-Agent: Mozilla/5.0 (X11; Linux i686; rv:13.0) Gecko/20120615 Thunderbird/13.0.1 MIME-Version: 1.0 To: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails References: <4FED9778.20001@bernhard-voelker.de> <4FEDA5DF.6000401@draigBrady.com> <4FEEE8A2.2020204@cs.ucla.edu> <4FEEFAEC.2020000@draigBrady.com> <4FF17632.4000601@draigBrady.com> <4FF22589.2000401@cs.ucla.edu> <4FF229D4.8030405@draigBrady.com> In-Reply-To: <4FF229D4.8030405@draigBrady.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -1.9 (-) X-Debbugs-Envelope-To: 11816 Cc: 11816@debbugs.gnu.org, Jim Meyering X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -1.9 (-) On 07/02/2012 04:08 PM, P=C3=A1draig Brady wrote: > Hmm I know I had EXIT_FAILURE here. > Should that be SORT_FAILURE? Yes, thanks, good catch. I pushed that as a further fix. From debbugs-submit-bounces@debbugs.gnu.org Tue Jul 03 04:30:44 2012 Received: (at 11816) by debbugs.gnu.org; 3 Jul 2012 08:30:44 +0000 Received: from localhost ([127.0.0.1]:44243 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SlyVQ-0008J6-Eb for submit@debbugs.gnu.org; Tue, 03 Jul 2012 04:30:44 -0400 Received: from mx.meyering.net ([88.168.87.75]:34519) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SlyVM-0008Ix-EE for 11816@debbugs.gnu.org; Tue, 03 Jul 2012 04:30:42 -0400 Received: from rho.meyering.net (rho.meyering.net [127.0.0.1]) by rho.meyering.net (Acme Bit-Twister) with ESMTP id 1E27560115; Tue, 3 Jul 2012 10:25:59 +0200 (CEST) From: Jim Meyering To: Paul Eggert Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails In-Reply-To: <4FF22589.2000401@cs.ucla.edu> (Paul Eggert's message of "Mon, 02 Jul 2012 15:49:45 -0700") References: <4FED9778.20001@bernhard-voelker.de> <4FEDA5DF.6000401@draigBrady.com> <4FEEE8A2.2020204@cs.ucla.edu> <4FEEFAEC.2020000@draigBrady.com> <4FF17632.4000601@draigBrady.com> <4FF22589.2000401@cs.ucla.edu> Date: Tue, 03 Jul 2012 10:25:59 +0200 Message-ID: <8739596yjs.fsf@rho.meyering.net> Lines: 20 MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -1.9 (-) X-Debbugs-Envelope-To: 11816 Cc: 11816@debbugs.gnu.org, =?iso-8859-1?Q?P=E1draig?= Brady X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -1.9 (-) Paul Eggert wrote: > Thanks for the improvement. > How about the following patch to simplify this a bit? > It removes a call to fdopen, among other things. > >>>From 05cc1b416a47330ef296dbeadd2a4b6095fe5c7d Mon Sep 17 00:00:00 2001 > From: Paul Eggert > Date: Mon, 2 Jul 2012 15:47:03 -0700 > Subject: [PATCH] sort: simplify -o handling to avoid fdopen, assert > > * src/sort.c (outfd): Remove. All uses replaced by STDOUT_FILENO. > (stream_open): When writing, use stdout rather than fdopen. > (move_fd_or_die): Renamed from dup2_or_die, with the added functionality > of closing its first argument. All uses changed. > (avoid_trashing_input): Special case for !outfile no longer needed. > (check_output): Arrange for standard output to go to the file, > rather than storing the fd in outfd. Nice further improvement. Thanks! From unknown Wed Jun 18 00:30:03 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Tue, 31 Jul 2012 11:24:03 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator