Re: [PATCH 2/2] blkid: introduce symbolic names for different blkid exit codes

[Karel Zak <kzak@redhat.com>]

On Wed, May 16, 2012 at 11:24:09AM +0200, Petr Uzel wrote:
> On Wed, May 16, 2012 at 10:52:40AM +0200, Voelker, Bernhard wrote:
> > Petr Uzel wrote:
> > 
> > >  #include "ismounted.h"
> > >  
> > > -#define STRTOXX_EXIT_CODE	4		/* strtoxx_or_err() */
> > > +#define STRTOXX_EXIT_CODE	BLKID_EXIT_OTHER	/* strtoxx_or_err() */
> > 
> > STRTOXX_EXIT_CODE is not used in blkid.c. Remove it?
> > ... or if it used in strutils.h, then this is not a nice interface.
> 
> Yep, it is used in strutils.h.

 Fortunately, STRTOXX_EXIT_CODE is rarely used (default is EXIT_FAILURE).

> > >  	if (rc == -2)
> > > -		return 8;	/* ambivalent probing result */
> > > +		return BLKID_EXIT_AMBIVAL;	/* ambivalent probing result */
> > >  	if (!nvals)
> > > -		return 2;	/* nothing detected */
> > > +		return BLKID_EXIT_NOTFOUND;	/* nothing detected */
> > >  
> > >  	return 0;		/* success */
> > >  }
> > 
> > These are not exit() codes, but return codes. 
> 
> Strictly speaking, yes. But the idea is that those codes are (or
> rather should be) propagated as exit value anyways...
> 
> > That value doesn't seem
> > to be of much interest anyway, because it's used only here in main()
> > in a for-loop, forgetting the return value of the previous iteration:
> > 
> >                for (i = 0; i < numdev; i++)
> >                         err = lowprobe_device(pr, devices[i], lowprobe, show,
> >                                         output_format,
> >                                         (blkid_loff_t) offset,
> >                                         (blkid_loff_t) size);
> 
> Hmm, the return value depends only on the last argument (device). I
> don't think this is intentional. Karel?
> 
>         # blkid -p /dev/sda /dev/sdb ; echo $?
>         /dev/sda: PTTYPE="gpt" 
>         error: /dev/sdb: No such file or directory
>         2
>         
>         # blkid -p /dev/sdb /dev/sda ; echo $?
>         error: /dev/sdb: No such file or directory
>         /dev/sda: PTTYPE="gpt" 
>         0

 It would be better to break the loop and exit after first error.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com