Can you please send a `git diff`?

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

_______________________________________________
parted-devel mailing list
[email protected]
http://lists.alioth.debian.org/mailman/listinfo/parted-devel

Reply via email to