Re: [PATCH 5/6] fdisk: introduce fdisk context

[Davidlohr Bueso <dave@xxxxxxx>]

On Mon, 2012-05-21 at 11:40 +0200, Karel Zak wrote:
> On Sun, May 20, 2012 at 06:11:36PM +0200, Davidlohr Bueso wrote:
> > +struct fdisk_context *fdisk_new_context_from_filename(const char *fname)
> > +{
> > +	int fd;
> > +	struct fdisk_context *cxt = NULL;
> > +
> > +	/*
> > +	 * Attempt to open the device with r-w permissions
> > +	 * by default, otherwise try read-only.
> > +	 */
> > +	if ((fd = open(fname, O_RDWR)) < 0)
> > +		if ((fd = open(fname, O_RDONLY)) < 0)
> > +			goto ret;
> 
>             return NULL;
> 
> > +	cxt = calloc(1, sizeof(*cxt));
> > +	if (!cxt)
> > +		goto ret;
> > +
> > +	cxt->dev_fd = fd;
> > +	cxt->dev_path = strdup(fname);
> > +	if (!cxt->dev_path)
> > +		return NULL;
> 
>         goto fail;
> 
>  Please, think about it as about library, it meas without memory and
>  fd leaks :-)
> 
> > +ret:
> > +	return cxt;
> > +}
> > +
> 
>    return cxt;
> 
> fail:
>    errsv = errno;
Hmm I hadn't considered saving errno stuff...

>    fdisk_free_context(cxt);

Yes, we need to plug that descriptor leak! fdisk_free_context() also
frees cxt->dev_path, so we should really just call close(fd).

Thanks,
Davidlohr

>    errno = errsv;
>    return NULL;
> 
> 
>     Karel
> 


--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html