On Wed, Aug 01, 2012 at 10:32:59AM -0700, Ansis Atteka wrote:
> On Wed, Aug 1, 2012 at 10:11 AM, Ben Pfaff <b...@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 <b...@nicira.com> wrote:
> > >
> > > > It will acquire its first user in an upcoming commit.
> > > >
> > > > Signed-off-by: Ben Pfaff <b...@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.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
