Hi Salvatore,
On Tue, 16 Aug 2011, Salvatore Bonaccorso wrote:
> Attached is a tentative patch trying to solve this: a link should only
> be updated, if it does not point to the right place.
Can you add a test-case for this? I guess that verifying that the inode
number did not change is a good way to verify that the link did not get
replaced... see utils/t/100_update_alternatives.t.
Also here are a few comments on your patch:
> --- a/utils/update-alternatives.c
> +++ b/utils/update-alternatives.c
> @@ -1645,7 +1645,9 @@ static void
> alternative_prepare_install_single(struct alternative *a, const char *name,
> const char *linkname, const char *file)
> {
> - char *fntmp, *fn;
> + char *fntmp, *fn, *lntarget;
> + bool alternative_needs_update = true;
> + struct stat st;
>
> /* Create link in /etc/alternatives. */
> xasprintf(&fntmp, "%s/%s" DPKG_TMP_EXT, altdir, name);
> @@ -1655,7 +1657,15 @@ alternative_prepare_install_single(struct alternative
> *a, const char *name,
> alternative_add_commit_op(a, opcode_mv, fntmp, fn);
> free(fntmp);
>
> - if (alternative_can_replace_path(linkname)) {
> + /* determine if alternative_needs_update, i.e. link already points to
> correct target */
> + if (lstat(linkname, &st) != -1) {
Better check "== 0" to match with the rest of the code. Also I would
suggest to put this whole check in a small static function for better
readability (similar to alternative_can_replace_path(), maybe
alternative_link_need_update(link, target)).
> + lntarget = xreadlink(linkname,false);
you have to free(lntarget) at some point since this is malloc()ed.
You miss a space after the comma.
> + if (S_ISLNK(st.st_mode) && strcmp(fn, lntarget) == 0) {
> + alternative_needs_update = false;
> + }
> + }
You need an else for the lstat() check where you should fail with an error
if the error is unexpected (aka if errno != ENOENT).
> +
> + if (alternative_can_replace_path(linkname) && alternative_needs_update)
> {
There's an else for this check that prints a warning, but we don't want to
print this warning if we skip the link creation because the link is
already ok.
Cheers,
--
Raphaël Hertzog ◈ Debian Developer
Follow my Debian News ▶ http://RaphaelHertzog.com (English)
▶ http://RaphaelHertzog.fr (Français)
--
To UNSUBSCRIBE, email to [email protected]
with a subject of "unsubscribe". Trouble? Contact [email protected]
