On Wed, Aug 1, 2012 at 10:11 AM, Ben Pfaff <[email protected]> wrote:

> On Tue, Jul 31, 2012 at 10:06:33AM -0700, Ansis Atteka wrote:
> > On Mon, Jul 30, 2012 at 3:18 PM, Ben Pfaff <[email protected]> wrote:
> >
> > > It will acquire its first user in an upcoming commit.
> > >
> > > Signed-off-by: Ben Pfaff <[email protected]>
>
> ...
>
> > > +xreadlink(const char *filename)
> > > +{
> > > +    size_t size;
> > >
> > +
> > > +    for (size = 64; ; size *= 2) {
> > > +        char *buf = xmalloc(size);
> > > +        int retval = readlink(filename, buf, size);
> > > +        int error = errno;
> > > +
> > > +        if (retval >= 0 && retval < size) {
> > > +            buf[retval] = '\0';
> > > +            return buf;
> > > +        }
> > > +
> > > +        free(buf);
> > > +        if (retval < size) {
> > >
> > Shouldn't size be of type ssize_t instead? Otherwise this could loop
> > forever, if readlink() returned -1.
>
> Good catch.  Thank you, that's a nasty bug.
>
> I think that changing (retval < size) to (retval < 0) fixes the
> problem too.  What do you think?
>
Yes, comparing with "0" actually would be better.

Also, I guess the type of retval should be ssize_t instead of int? I guess
this was changed starting from SUSv3.

Did you consider to use constant PATH_MAX or SYMLINK_MAX? Anyway,
I am completely fine with your solution too, because I think that the
path in majority of cases would fit in a 64 byte buffer anyway.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to