Peter Baumann <waste.manager@xxxxxx> wrote:
> On Tue, Aug 07, 2012 at 08:45:10PM +0000, Eric Wong wrote:
> > Peter Baumann <waste.manager@xxxxxx> wrote:
> > > + for my $suffix (qw(yaml db)) {
> > > + unlink("$cache_file.$suffix");
> >
> > Need to check for unlink() errors (and ignore ENOENT).
>
> I'm not sure what you mean here: Aren't we screwed either way if unlinking
> the file failed? There is nothhing we can do about it if e.g. the user doesn't
> have the permissions to delete the file, besides terminating, e.g.
>
> for my $cache_file (("$cache_path/lookup_svn_merge",
> "$cache_path/check_cherry_pick",
> "$cache_path/has_no_changes")) {
> for my $suffix (qw(yaml db)) {
> next unless (-e "$cache_file.$suffix");
> unlink("$cache_file.$suffix") or
> die "Failed to delete $cache_file.$suffix";
> }
Yes we're screwed, but silent failure is the worst way to fail,
especially if it can lead us back to the problems your patch is meant to
address.
Perhaps something like this (with $! to show the error):
my $file = "$cache_file.$suffix";
next unless -e $file;
unlink($file) or die "unlink($file) failed: $!\n";
--
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