git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Rast <trast@student.ethz.ch>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Martin Ettl <ettl.martin@gmx.de>, git@vger.kernel.org
Subject: Re: found a resource leak in file builtin-fast-export.c
Date: Thu, 9 Jul 2009 13:24:14 +0200	[thread overview]
Message-ID: <200907091324.17643.trast@student.ethz.ch> (raw)
In-Reply-To: <alpine.DEB.1.00.0907091302520.4339@intel-tinevez-2-302>

[-- Attachment #1: Type: Text/Plain, Size: 918 bytes --]

Johannes Schindelin wrote:
> On Thu, 9 Jul 2009, Thomas Rast wrote:
> 
> > Martin Ettl wrote:
> > > -	if (ferror(f) || fclose(f))
> > > +	if (ferror(f))
> > >  		error("Unable to write marks file %s.", file);
> > > +  	fclose(f);
> > 
> > You no longer check the error returned by fclose().  This is
> > important, because the FILE* API may buffer writes, and a write error
> > may only become apparent when fclose() flushes the file.
> 
> Indeed.  A better fix would be to replace the || by a |, but this must be 
> accompanied by a comment so it does not get removed due to overzealous 
> compiler warnings.

Are you allowed to do that?  IIRC using | no longer guarantees that
ferror() is called before fclose(), and my local 'man 3p fclose' says
that

       After the call to fclose(), any use of stream results in
       undefined behavior.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

  reply	other threads:[~2009-07-09 11:24 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-09  7:57 found a resource leak in file builtin-fast-export.c Martin Ettl
2009-07-09  8:31 ` Thomas Rast
2009-07-09 11:04   ` Johannes Schindelin
2009-07-09 11:24     ` Thomas Rast [this message]
2009-07-09 11:30       ` Andreas Ericsson
2009-07-09 13:01       ` Johannes Schindelin
2009-07-09 13:28         ` [PATCH] Fix export_marks() error handling Matthias Andree
2009-07-11  9:45           ` Stephen R. van den Berg
2009-07-13  8:01             ` Matthias Andree
2009-07-09 13:36         ` found a resource leak in file builtin-fast-export.c Matthias Andree

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200907091324.17643.trast@student.ethz.ch \
    --to=trast@student.ethz.ch \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=ettl.martin@gmx.de \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).