[libvirt] [PATCH 02/10] util: Introduce virFileReadLink

[Daniel P. Berrange]

On Thu, Feb 02, 2017 at 03:11:56PM +0100, Martin Kletzander wrote:
> On Fri, Jan 20, 2017 at 10:42:42AM +0100, Michal Privoznik wrote:
> > We will need to traverse the symlinks one step at the time.
> > Therefore we need to see where a symlink is pointing to.
> > 
> > Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> > ---
> > src/libvirt_private.syms |  1 +
> > src/util/virfile.c       | 12 ++++++++++++
> > src/util/virfile.h       |  2 ++
> > 3 files changed, 15 insertions(+)
> > 
> > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> > index cfeb43cf0..7f7dcfe44 100644
> > --- a/src/libvirt_private.syms
> > +++ b/src/libvirt_private.syms
> > @@ -1615,6 +1615,7 @@ virFileReadAllQuiet;
> > virFileReadBufQuiet;
> > virFileReadHeaderFD;
> > virFileReadLimFD;
> > +virFileReadLink;
> > virFileRelLinkPointsTo;
> > virFileRemove;
> > virFileRemoveLastComponent;
> > diff --git a/src/util/virfile.c b/src/util/virfile.c
> > index bf8099e34..49ea1d1f0 100644
> > --- a/src/util/virfile.c
> > +++ b/src/util/virfile.c
> > @@ -76,6 +76,7 @@
> > #include "virutil.h"
> > 
> > #include "c-ctype.h"
> > +#include "areadlink.h"
> > 
> > #define VIR_FROM_THIS VIR_FROM_NONE
> > 
> > @@ -1614,6 +1615,17 @@ virFileIsLink(const char *linkpath)
> >     return S_ISLNK(st.st_mode) != 0;
> > }
> > 
> > +/*
> > + * Read where symlink is pointing to.
> > + *
> > + * Returns 0 on success (@linkpath is a successfully read link),
> > + *        -1 with errno set upon error.
> > + */
> > +int
> > +virFileReadLink(const char *linkpath, char **resultpath)
> > +{
> > +    return (*resultpath = areadlink(linkpath)) ? 0 : -1;
> > +}
> > 
> 
> I don't really see the benefit of this function, are you trying to
> obfuscate it?  You essentially double the return information for no
> reason and try to push it all into one line.

It would be useful if you have an ATTRIBUTE_RETURN_CHECK annotation
on it, as it would allow compiler time verification of error checking,
which is not possible when you overload the return value to contain
both the data and error indicator.

> > /*
> >  * Finds a requested executable file in the PATH env. e.g.:
> > diff --git a/src/util/virfile.h b/src/util/virfile.h
> > index 0343acd5b..981a9e07d 100644
> > --- a/src/util/virfile.h
> > +++ b/src/util/virfile.h
> > @@ -166,6 +166,8 @@ int virFileResolveAllLinks(const char *linkpath,
> > int virFileIsLink(const char *linkpath)
> >     ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
> > 
> > +int virFileReadLink(const char *linkpath, char **resultpath);

eg add ATTRIBUTE_RETURN_CHECK here

> > +
> > char *virFindFileInPath(const char *file);
> > 
> > char *virFileFindResource(const char *filename,


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|