[libvirt] [PATCH] nodeinfo: Make sure we always reset errno before calling readdir

[Daniel P. Berrange]

On Thu, Apr 10, 2014 at 08:24:20AM -0600, Eric Blake wrote:
> On 04/10/2014 08:04 AM, Natanael Copa wrote:
> > We must always reset errno to 0 even if we do 'continue'.
> > 
> > This fixes runtime with musl libc which will set errno on sscanf.
> > 
> > Signed-off-by: Natanael Copa <ncopa at alpinelinux.org>
> > ---
> >  src/nodeinfo.c | 15 +++------------
> >  1 file changed, 3 insertions(+), 12 deletions(-)
> > 
> > diff --git a/src/nodeinfo.c b/src/nodeinfo.c
> > index 53ba716..8d3214e 100644
> > --- a/src/nodeinfo.c
> > +++ b/src/nodeinfo.c
> > @@ -452,8 +452,7 @@ virNodeParseNode(const char *node,
> >  
> >      /* enumerate sockets in the node */
> >      CPU_ZERO(&sock_map);
> > -    errno = 0;
> > -    while ((cpudirent = readdir(cpudir))) {
> > +    for (errno = 0; (cpudirent = readdir(cpudir)); errno = 0) {
> >          if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
> >              continue;
> 
> Good catch.  However, the code is still buggy even after your fix.  We
> are missing:
> 
> if (errno)
>     break;
> 
> before attempting the sscanf.  Furthermore, sscanf() is undefined on
> overflow; while the cpudir is unlikely to be giving us integers that
> overflow, it would be nicer to not use sscanf in the first place.  It's
> more than just the improper use of readdir that needs fixing here.
> 
> I'm debating whether to push this now and fix the rest as a followup, or
> whether to do a v2 that fixes it all at once.

More generally, just about every use of readdir() in our codebase is
broken in this way. IMHO it is time to create  virFileReaddir() which
wraps it and doesn't overload the return value to include both error
and data values ie use an output parameter for the returned dir
instance, and raise full libvirt errors upon error.


Regards,
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 :|