Re: [pbs-devel] [PATCH v2 proxmox-backup 1/2] fix: #4761: unlink existing entries for hard/symlinks when overwrite

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Fri, Aug 04, 2023 at 02:20:40PM +0200, Wolfgang Bumiller wrote:
> On Tue, Aug 01, 2023 at 12:34:11PM +0200, Christian Ebner wrote:
> > Creating symlinks or hardlinks might fail if a directory entry with the
> > same name is already present on the filesystem during restore.
> > 
> > When the overwrite flag is given, on failure unlink the existing entry
> > (except directories) and retry hard/symlink creation.
> > 
> > Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> > ---
> > changes since v1:
> > * rebased to current master
> > 
> >  pbs-client/src/pxar/extract.rs | 39 +++++++++++++++++++++++++++++++---
> >  1 file changed, 36 insertions(+), 3 deletions(-)
> > 
> > diff --git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs
> > index 4dbaf52d..c1e7b417 100644
> > --- a/pbs-client/src/pxar/extract.rs
> > +++ b/pbs-client/src/pxar/extract.rs
> > @@ -547,7 +547,21 @@ impl Extractor {
> >          link: &OsStr,
> >      ) -> Result<(), Error> {
> >          let parent = self.parent_fd()?;
> > -        nix::unistd::symlinkat(link, Some(parent), file_name)?;
> > +
> > +        match nix::unistd::symlinkat(link, Some(parent), file_name) {
> > +            Ok(()) => {}
> > +            Err(nix::errno::Errno::EEXIST) => {
> > +                if !self.overwrite {
> > +                    return Err(nix::errno::Errno::EEXIST.into());
> > +                }
> 
> The above could be shortened to a pattern gaurd:
> 
>                Err(nix::errno::Errno::EEXIST) if self.overwrite => {
> 
> then we don't need the extra `return Err()` since we just run into the
> final `Err(err)` case.

As a side note, you could have used
    Err(err @ nix::errno::Errno::EEXIST)
and then returned `Err(err.into())` ;-)