Re: [PATCH 1/2] libxl: fix _SC_GETPW_R_SIZE_MAX usage

[Ian Campbell <ian.campbell@citrix.com>]

On Wed, 2016-01-13 at 10:08 +0100, Roger Pau Monné wrote:
> El 13/01/16 a les 4.19, Doug Goldstein ha escrit:
> > On 1/12/16 7:14 AM, Roger Pau Monne wrote:
> > > According to the FreeBSD sysconf man page [0] if the variable is
> > > associated
> > > with functionality that is not supported, -1 is returned and errno is
> > > not
> > > modified. Modify libxl__dm_runas_helper so it's able to correctly
> > > deal with this situation by setting the initial buffer value to 2048.
> > > 
> > > [0] https://www.freebsd.org/cgi/man.cgi?query=sysconf

http://pubs.opengroup.org/onlinepubs/9699919799/functions/sysconf.html (whi
ch IMHO is the more canonical documentation) describes a similar error
behaviour, although for a subtly different case (no limit for the given
variable).

> > > 
> > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > > ---
> > >  tools/libxl/libxl_dm.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > > index 0aaefd9..ec8fb51 100644
> > > --- a/tools/libxl/libxl_dm.c
> > > +++ b/tools/libxl/libxl_dm.c
> > > @@ -728,7 +728,14 @@ static int libxl__dm_runas_helper(libxl__gc *gc,
> > > const char *username)
> > >      long buf_size;
> > >      int ret;
> > >  
> > > +    errno = 0;
> > >      buf_size = sysconf(_SC_GETPW_R_SIZE_MAX);
> > > +    if (buf_size < 0 && errno == 0) {
> > > +        buf_size = 2048;
> > > +        LOG(DEBUG,
> > > +"sysconf(_SC_GETPW_R_SIZE_MAX) is not supported, using a buffer size of %ld",

"... an initial buffer size of ..."

> > > +            buf_size);
> > > +    }
> > >      if (buf_size < 0) {
> > >          LOGE(ERROR, "sysconf(_SC_GETPW_R_SIZE_MAX) returned error
> > > %ld",
> > >                  buf_size);
> > > 
> > 
> > So on Linux the behavior is somewhat similar [1]. But I took a peek at
> > the libvirt code for doing the similar thing and I notice that they
> > just
> > default if the value is returned as less than 0 [2]. Reading both man
> > pages it seems like that would be the better bet instead of failing how
> > the current code is. Your fix probably makes it fail less but it could
> > still error out senselessly. Just a suggestion for an improvement
> > overall.
> 
> Ack, it doesn't make much sense to error out if we cannot find an
> initial value for the buffer, the code below is able to adapt and expand
> the buffer as needed anyway. I'm going to resend with that fixed.

Sounds good.

Don't forget to CC all the tools maintainers next time.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel