From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6411 invoked by alias); 7 Jan 2015 06:35:38 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 34153 Received: (qmail 16763 invoked from network); 7 Jan 2015 06:35:26 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 X-CMAE-Score: 0 X-CMAE-Analysis: v=2.1 cv=CoYIqc8G c=1 sm=1 tr=0 a=FT8er97JFeGWzr5TCOCO5w==:117 a=kj9zAlcOel0A:10 a=q2GGsy2AAAAA:8 a=oR5dmqMzAAAA:8 a=-9mUelKeXuEA:10 a=YNv0rlydsVwA:10 a=4IP8-eEMscBup92aspgA:9 a=KobkiUlm7PPYRnNx:21 a=B5SMG45MTCigbGfv:21 a=CjuIK1q_8ugA:10 From: Bart Schaefer Message-id: <150106223532.ZM1050@torch.brasslantern.com> Date: Tue, 06 Jan 2015 22:35:31 -0800 In-reply-to: <1420590318-17047-1-git-send-email-mikachu@gmail.com> Comments: In reply to Mikael Magnusson "PATCH: Plug some fd leaks in bin_print" (Jan 7, 1:25am) References: <1420590318-17047-1-git-send-email-mikachu@gmail.com> X-Mailer: OpenZMail Classic (0.9.2 24April2005) To: zsh-workers@zsh.org Subject: Re: PATCH: Plug some fd leaks in bin_print MIME-version: 1.0 Content-type: text/plain; charset=us-ascii I have some doubts about some of this. In fact I suspect that these are mostly false-positives because the option letters tested in the if() conditions preceding each of these goto's are (or should be) mutually exclusive. On Jan 7, 1:25am, Mikael Magnusson wrote: } } if (*eptr) { } - zwarnnam(name, "number expcted after -%c: %s", 'C', argptr); } - return 1; } + zwarnnam(name, "number expected after -%c: %s", 'C', argptr); } + ret = 1; } + goto out_print; } } } if (nc <= 0) { } zwarnnam(name, "invalid number of columns: %s", argptr); } - return 1; } + ret = 1; } + goto out_print; } } I'm not sure what coverity had to say here, but these can't be right. The descriptor should NOT be fflush()d on this error. } queue_signals(); } zpushnode(bufstack, sepjoin(args, NULL, 0)); } unqueue_signals(); } - return 0; } + goto out_print; This can't be right either; there shouldn't be any output when pushing on the buffer stack, so again we shouldn't be fflush()ing. } zwarnnam(name, "option -S takes a single argument"); } - return 1; } + ret = 1; } + goto out_print; Same complaint. } if (OPT_ISSET(ops,'z') || OPT_ISSET(ops,'s')) { } + if (fout != stdout) } + fclose(fout); } + fout = stdout; Why is that needed when the very next couple of lines are going to over- write fout with a different open? I'm just going to quit complaining about the rest of these; I think they're all incorrect. If nothing else, they shouldn't introduce the possibility of printing BOTH an argument-parsing error AND a write- failed error. This needs to be looked at more closely. -- Barton E. Schaefer