On Wed, Nov 26, 2008 at 05:22:05AM -0500, Joel Granados wrote:
> Can you please send a `git diff`?
I actually do not have one - since I'am not sure about the patch I
wanted to discuss what/how has to be changed :)
>
> Regards
> ----- "Petr Uzel" <[EMAIL PROTECTED]> wrote:
>
> > Hi list!
> >
> > 1 PedExceptionOption
> > 2 ped_exception_throw (PedExceptionType ex_type,
> > 3 PedExceptionOption ex_opts, const char* message,
> > ...)
> > 4 {
> > 5 va_list arg_list;
> > 6 int result;
> > 7 static int size = 1000;
> >
> > 8 if (ex)
> > 9 ped_exception_catch ();
> >
> > 10 ex = (PedException*) malloc (sizeof (PedException));
> > 11 if (!ex)
> > 12 goto no_memory;
> >
> > 13 ex->type = ex_type;
> > 14 ex->options = ex_opts;
> >
> > 15 while (message) {
> > 16 ex->message = (char*) malloc (size * sizeof
> > (char));
> > 17 if (!ex->message)
> > 18 goto no_memory;
> >
> > 19 va_start (arg_list, message);
> > 20 result = vsnprintf (ex->message, size, message,
> > arg_list);
> > 21 va_end (arg_list);
> >
> > 22 if (result > -1 && result < size)
> > 23 break;
> >
> > 24 size += 10;
> > 25 free (ex->message);
> > 26 }
> >
> > 27 return do_throw ();
> >
> > If this function gets NULL in 'message' parameter, it will go into
> > endless loop allocating memory because vsnprintf() on line 20 will
> > keep returning -1 and thus the condition on line 22 will never be
> > true.
> >
> > There is at least one place where ped_exception_throw() is called
> > with
> > NULL : libparted/labels/dasd.c:243 [*]
> >
> > I suggest to:
> > a) PED_ASSERT(message != NULL) somewhere in ped_exception_throw()
> > b) change the condition on line 22 to be just 'if (result < size)',
> > because when vsnprintf() once returned -1, it will probably keep
> > returning the same even with larger buffer.
> > c) fix [*] to something more appropriate, such as:
> >
> > ped_exception_throw(PED_EXCEPTION_ERROR, PED_EXCEPTION_OK,
> > _("%s is corrupted"),
> > dev->path);
> >
> > Of course the 'is corrupted' text has to be changed to something
> > better.
> >
> > BTW isn't the while loop a typical case where realloc() would be more
> > appropriate then malloc()/free() ?
> >
> >
> > What do you guys think?
> >
> >
> > --
> > Best regards / s pozdravem
> >
> > Petr Uzel, Packages maintainer
> > ---------------------------------------------------------------------
> > SUSE LINUX, s.r.o. e-mail: [EMAIL PROTECTED]
> > Lihovarská 1060/12 tel: +420 284 028 964
> > 190 00 Prague 9 fax: +420 284 028 951
> > Czech Republic http://www.suse.cz
> >
> > _______________________________________________
> > parted-devel mailing list
> > [email protected]
> > http://lists.alioth.debian.org/mailman/listinfo/parted-devel
>
> --
> Joel Andres Granados
> Red Hat / Brno Czech Republic
--
Best regards / s pozdravem
Petr Uzel, Packages maintainer
---------------------------------------------------------------------
SUSE LINUX, s.r.o. e-mail: [EMAIL PROTECTED]
Lihovarská 1060/12 tel: +420 284 028 964
190 00 Prague 9 fax: +420 284 028 951
Czech Republic http://www.suse.cz
_______________________________________________
parted-devel mailing list
[email protected]
http://lists.alioth.debian.org/mailman/listinfo/parted-devel
