[libvirt] [PATCH 1/3] libvirt-domain: Introduce macro to save duplicated codes

[Wei, Jiangang]

On Thu, 2015-10-15 at 11:31 +0200, Peter Krempa wrote:
> On Thu, Oct 15, 2015 at 17:12:18 +0800, Wei Jiangang wrote:
> > Signed-off-by: Wei Jiangang <weijg.fnst at cn.fujitsu.com>
> > ---
> >  src/libvirt-domain.c | 55 ++++++++++++++--------------------------------------
> >  1 file changed, 15 insertions(+), 40 deletions(-)
> > 
> > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> > index 6e1aacd..ed07c9e 100644
> > --- a/src/libvirt-domain.c
> > +++ b/src/libvirt-domain.c
> > @@ -33,6 +33,13 @@ VIR_LOG_INIT("libvirt.domain");
> >  
> >  #define VIR_FROM_THIS VIR_FROM_DOMAIN
> >  
> > +#define VIR_ABSOLUTIZE_PATH(PATH, ABSPATH)                             \
> > +    if (virFileAbsPath(PATH, ABSPATH) < 0) {                           \
> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",                   \
> > +                       _("could not build absolute input file path")); \
> 
> This says "input" file path ... 
> 
> > +        goto error;                                                    \
> > +    }
> > +
> >  
> >  /**
> >   * virConnectListDomains:
> > @@ -830,11 +837,7 @@ virDomainSave(virDomainPtr domain, const char *to)
> >          char *absolute_to;
> >  
> >          /* We must absolutize the file path as the save is done out of process */
> > -        if (virFileAbsPath(to, &absolute_to) < 0) {
> > -            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > -                           _("could not build absolute output file path"));
> 
> But this "output" ...
> 
> > -            goto error;
> > -        }
> > +        VIR_ABSOLUTIZE_PATH(to, &absolute_to);
> >  
> >          ret = conn->driver->domainSave(domain, absolute_to);
> >  
> 
> 
> > @@ -968,11 +967,7 @@ virDomainRestore(virConnectPtr conn, const char *from)
> >          char *absolute_from;
> >  
> >          /* We must absolutize the file path as the restore is done out of process */
> > -        if (virFileAbsPath(from, &absolute_from) < 0) {
> > -            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > -                           _("could not build absolute input file path"));
> 
> While here correctly "input"
> 
> > -            goto error;
> > -        }
> 
> While saving code, this would actually break the error messages in some
> cases.
Thanks for your comments.
If I make some adjustment like below,
Is it acceptable?

+#define VIR_ABSOLUTIZE_PATH(PATH, ABSPATH, ERRSTR)
\
+    if (virFileAbsPath(PATH, ABSPATH) < 0) {
\
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", ERRSTR);
\
+        goto error;
\
+    }
+

-        if (virFileAbsPath(to, &absolute_to) < 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("could not build absolute output file
path"));
-            goto error;
-        }
+        VIR_ABSOLUTIZE_PATH(to, &absolute_to,
+                            _("could not build absolute output file
path"));

Regards,
wei

> 
> Peter