rpki-client rrdp merge repo fix

[Claudio Jeker]

In some cases unlink reports 'no such file or directory' when the RRDP
repository is merged at the end of a RRDP sync.
The problem is that some deleted files are in the temporary location and
not part of the real repo. Because of this if unlink return ENOENT then
try the alternate location.

While there cleanup the code a bit more and make the rename() and unlink()
case similar when it comes to the path generation bits.

With this I no longer see the unlink errors.
OK?
--
:wq Claudio

Index: repo.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v
retrieving revision 1.3
diff -u -p -r1.3 repo.c
--- repo.c 2 Apr 2021 05:16:29 -0000 1.3
+++ repo.c 7 Apr 2021 09:01:59 -0000
@@ -838,22 +838,37 @@ rrdp_merge_repo(struct rrdprepo *rr)
 
  /* XXX should delay deletes */
  RB_FOREACH_SAFE(fp, filepath_tree, &rr->deleted, nfp) {
- if ((fn = rrdp_filename(rr, fp->file, 0)) != NULL) {
- if (unlink(fn) == -1)
- warn("%s: unlink", fn);
- free(fn);
+ fn = rrdp_filename(rr, fp->file, 1);
+ rfn = rrdp_filename(rr, fp->file, 0);
+
+ if (fn == NULL || rfn == NULL)
+ errx(1, "bad filepath"); /* should not happen */
+
+ if (unlink(rfn) == -1) {
+ if (errno == ENOENT) {
+ if (unlink(fn) == -1)
+ warn("%s: unlink", fn);
+ } else
+ warn("%s: unlink", rfn);
  }
+
+ free(rfn);
+ free(fn);
  filepath_put(&rr->deleted, fp);
  }
 
  RB_FOREACH_SAFE(fp, filepath_tree, &rr->added, nfp) {
- if ((fn = rrdp_filename(rr, fp->file, 1)) != NULL &&
-    (rfn = rrdp_filename(rr, fp->file, 0)) != NULL) {
- repo_mkpath(rfn);
- if (rename(fn, rfn) == -1)
- warn("%s: link", rfn);
- free(rfn);
- }
+ fn = rrdp_filename(rr, fp->file, 1);
+ rfn = rrdp_filename(rr, fp->file, 0);
+
+ if (fn == NULL || rfn == NULL)
+ errx(1, "bad filepath"); /* should not happen */
+
+ repo_mkpath(rfn);
+ if (rename(fn, rfn) == -1)
+ warn("%s: rename", rfn);
+
+ free(rfn);
  free(fn);
  filepath_put(&rr->added, fp);
  }

[Theo Buehler-3]

On Wed, Apr 07, 2021 at 12:50:15PM +0200, Claudio Jeker wrote:

> In some cases unlink reports 'no such file or directory' when the RRDP
> repository is merged at the end of a RRDP sync.
> The problem is that some deleted files are in the temporary location and
> not part of the real repo. Because of this if unlink return ENOENT then
> try the alternate location.
>
> While there cleanup the code a bit more and make the rename() and unlink()
> case similar when it comes to the path generation bits.
>
> With this I no longer see the unlink errors.
> OK?

ok tb

> --
> :wq Claudio
>
> Index: repo.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 repo.c
> --- repo.c 2 Apr 2021 05:16:29 -0000 1.3
> +++ repo.c 7 Apr 2021 09:01:59 -0000
> @@ -838,22 +838,37 @@ rrdp_merge_repo(struct rrdprepo *rr)
>  
>   /* XXX should delay deletes */
>   RB_FOREACH_SAFE(fp, filepath_tree, &rr->deleted, nfp) {
> - if ((fn = rrdp_filename(rr, fp->file, 0)) != NULL) {
> - if (unlink(fn) == -1)
> - warn("%s: unlink", fn);
> - free(fn);
> + fn = rrdp_filename(rr, fp->file, 1);
> + rfn = rrdp_filename(rr, fp->file, 0);
> +
> + if (fn == NULL || rfn == NULL)
> + errx(1, "bad filepath"); /* should not happen */
> +
> + if (unlink(rfn) == -1) {
> + if (errno == ENOENT) {
> + if (unlink(fn) == -1)
> + warn("%s: unlink", fn);
> + } else
> + warn("%s: unlink", rfn);
>   }
> +
> + free(rfn);
> + free(fn);
>   filepath_put(&rr->deleted, fp);
>   }
>  
>   RB_FOREACH_SAFE(fp, filepath_tree, &rr->added, nfp) {
> - if ((fn = rrdp_filename(rr, fp->file, 1)) != NULL &&
> -    (rfn = rrdp_filename(rr, fp->file, 0)) != NULL) {
> - repo_mkpath(rfn);
> - if (rename(fn, rfn) == -1)
> - warn("%s: link", rfn);
> - free(rfn);
> - }
> + fn = rrdp_filename(rr, fp->file, 1);
> + rfn = rrdp_filename(rr, fp->file, 0);
> +
> + if (fn == NULL || rfn == NULL)
> + errx(1, "bad filepath"); /* should not happen */
> +
> + repo_mkpath(rfn);
> + if (rename(fn, rfn) == -1)
> + warn("%s: rename", rfn);
> +
> + free(rfn);
>   free(fn);
>   filepath_put(&rr->added, fp);
>   }
>