Hello Guillem,
On Fri, 26 Aug 2011, Guillem Jover wrote:
> The other day while I was messing with u-a I prepared the attached patch
> which I think is better (but actually pretty close to what you have
> now here :). But forgot including it in my push. In any case I'll
> include it for my next one.
Hum, I think I found a bug in your code. And some details too:
> +static bool
> +alternative_path_needs_update(const char *linkname, const char *filename)
> +{
> + char *linktarget;
> + bool update;
> +
> + if (opt_force)
> + return true;
This early-return clause doesn't make sense, it seems to be there only
because you do no want to run xreadlink() on something that might not be a
symlink. If that's the case, a comment explaining it would be good.
Unless you want --force to redo something that's not needed, but I don't
really see why you'd want that.
IMO you could just as well drop it entirely since you call
xreadlink(linkname, false) (i.e. it can't error out, but it will return
NULL in case of failure).
> + linktarget = xreadlink(linkname, false);
There's a real possibility that linktarget is NULL when linkname doesn't
exist. That's one of the usual case where can_replace_path() will return
true and where path_needs_update() will thus be called.
> + if (strcmp(linktarget, filename) == 0)
Thus you should protect the call here:
if (linktarget && strcmp(…) == 0)
> + update = false;
> + else
> + update = true;
> + free(linktarget);
> +
> + return update;
> +}
That's the only problem I could spot.
Cheers,
--
Raphaël Hertzog ◈ Debian Developer
Follow my Debian News ▶ http://RaphaelHertzog.com (English)
▶ http://RaphaelHertzog.fr (Français)
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
