[ovs-dev] [debian 7/9] util: New function follow_symlinks().

Ben Pfaff blp at nicira.com
Wed Aug 1 17:53:11 UTC 2012


On Wed, Aug 01, 2012 at 10:32:59AM -0700, Ansis Atteka wrote:
> On Wed, Aug 1, 2012 at 10:11 AM, Ben Pfaff <blp at nicira.com> 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 <blp at nicira.com> wrote:
> > >
> > > > It will acquire its first user in an upcoming commit.
> > > >
> > > > Signed-off-by: Ben Pfaff <blp at nicira.com>
> >
> > ...
> >
> > > > +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.

Thanks, changed.

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

Thanks, changed.

> 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.

PATH_MAX is always tempting, but POSIX says:

    The values in the following list may be constants within an
    implementation or may vary from one pathname to another. For
    example, file systems or directories may have different
    characteristics.

    A definition of one of the symbolic constants in the following
    list shall be omitted from the <limits.h> header on specific
    implementations where the corresponding value is equal to or
    greater than the stated minimum, but where the value can vary
    depending on the file to which it is applied. The actual value
    supported for a specific pathname shall be provided by the
    pathconf() function.

which means that you end up having to call pathconf() in some cases.
And some systems don't really have a limit anyway.

GNU Hurd is an example of a system where PATH_MAX causes problems:
        http://www.gnu.org/software/hurd/community/gsoc/project_ideas/maxpath.html

In the extreme, you end up with nastiness like this:
        http://git.savannah.gnu.org/cgit/gnulib.git/tree/lib/pathmax.h

So I prefer to just use dynamic allocation, because to me it seems
cleaner.

Thanks,

Ben.



More information about the dev mailing list