[libvirt] [PATCH 05/13] Convert (nearly) all files in src/util/ to use virReportError()

[Daniel P. Berrange]

On Wed, Jul 18, 2012 at 07:54:11AM -0600, Eric Blake wrote:
> > +++ b/src/util/hooks.c
> 
> > @@ -252,9 +248,9 @@ virHookCall(int driver,
> >              break;
> >      }
> >      if (opstr == NULL) {
> > -        virHookReportError(VIR_ERR_INTERNAL_ERROR,
> > -                           _("Hook for %s, failed to find operation #%d"),
> > -                           drvstr, op);
> > +        virReportError(VIR_ERR_INTERNAL_ERROR,
> > +                       _("Hook for %s, failed to find operation #%d"),
> 
> Generic question - now that we are touching 90% of the existing error
> messages, should we establish a convention on whether all messages
> should start with a lower case letter?  (That would at least make us
> consistent with GNU Coding Standards, and while we are not a GNU
> project, cross-project consistency does have an advantage.  Personally,
> I've noticed our inconsistent use of capitals for more than 2 years now,
> but it hasn't been high enough on my "itch radar" to scratch it.)
> Obviously, a syntax check would be needed IF we make a decision to
> standardize.

Yeah sounds like a reasonble idea to fix this.

> > +++ b/src/util/pci.c
> > @@ -744,7 +740,7 @@ pciResetDevice(pciDevice *dev,
> >      int ret = -1;
> >  
> >      if (activeDevs && pciDeviceListFind(activeDevs, dev)) {
> > -        pciReportError(VIR_ERR_INTERNAL_ERROR,
> > +        virReportError(VIR_ERR_INTERNAL_ERROR,
> >                         _("Not resetting active device %s"), dev->name);
> 
> Code like this means we will probably see a second round of patches in
> the future for scrubbing stupid uses of VIR_ERR_INTERNAL_ERROR into
> better messages for user-visible errors :)

Arugably nearly every use of VIR_ERR_INTERNAL_ERROR ought to
be replaced by something more meaningful. That's not a job I
wish to tackle now :-)

> 
> > +++ b/src/util/stats_linux.c
> > @@ -106,8 +102,8 @@ linuxDomainInterfaceStats(const char *path,
> >      }
> >      VIR_FORCE_FCLOSE(fp);
> >  
> > -    virStatsError(VIR_ERR_INTERNAL_ERROR,
> > -                  _("/proc/net/dev: Interface not found"));
> > +    virReportError(VIR_ERR_INTERNAL_ERROR,
> > +                   _("/proc/net/dev: Interface not found"));
> 
> Needs a "%s" argument to silence compiler warnings when i18n is disabled.

I'm fixing this as a separate patch, since this was a pre-existing
problem

> 
> > +++ b/src/util/util.c
> > @@ -333,8 +329,8 @@ virPipeReadUntilEOF(int outfd, int errfd,
> >                  if (fds[i].revents & POLLHUP)
> >                      continue;
> >  
> > -                virUtilError(VIR_ERR_INTERNAL_ERROR,
> > -                             "%s", _("Unknown poll response."));
> > +                virReportError(VIR_ERR_INTERNAL_ERROR,
> > +                               "%s", _("Unknown poll response."));
> 
> Is it worth scrubbing for error messages that end with '.'?  That's
> another thing that GNU Coding Standards discourage.

Something to fix at the same time as the capitalization

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|