Re: [PATCH v5 03/35] rollback_lock_file(): do not clear filename redundantly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Michael Haggerty wrote:

> It is only necessary to clear the lock_file's filename field if it was
> not already clear.
[...]
> --- a/lockfile.c
> +++ b/lockfile.c
> @@ -276,6 +276,6 @@ void rollback_lock_file(struct lock_file *lk)
>  		if (lk->fd >= 0)
>  			close(lk->fd);
>  		unlink_or_warn(lk->filename);
> +		lk->filename[0] = 0;
>  	}
> -	lk->filename[0] = 0;

Now that it does nothing when lk->filename[0] == '\0', this could be
de-indented a little:

	if (!lk->filename[0])
		return;
	if (lk->fd >= 0)
		close(lk->fd);
	unlink_or_warn(lk->filename);
	lk->filename[0] = 0;

This could technically double-close if interrupted by a signal that
tries to remove the file again, right?  Should this use
close_lock_file which sets fd to -1 before closing?

	if (!lk->filename[0])
		return;
	close_lock_file(lk);
	unlink_or_warn(lk->filename);
	lk->filename[0] = 0;

With or without such changes,
Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]