rpki-client rrdp merge repo fix

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

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);
  }

Reply | Threaded
Open this post in threaded view
|

Re: rpki-client rrdp merge repo fix

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);
>   }
>