Re: [pdf-devel] New developer and UUID module request

[Albert Meroño Peñuela]

Hi,

Please find attached a new patch. I've considered all your comments
and implemented the string issue with the c) option. I've also updated
the documentation. Thank you Jose and Aleksander for your reviews :-)

Cheers,

Albert

2011/3/22 Aleksander Morgado <address@hidden>:
> Hi Albert,
>
>> const char * pdf_uuid_string (pdf_uuid_t uuid, char
>> ascii_rep[PDF_UUID_CHAR_LENGTH]);
>>
>> is thread-safe and working well. The function returns the same pointer
>> it gets as input parameter (ascii_rep). The user is still required to
>> initialize the char array, but the size he or she specifies seems to
>> have no effect; the following testing code worked fine to me, writing
>> PDF_UUID_CHAR_LENGTH chars in stdout:
>>
>> pdf_uuid_t t;
>> char t_a[1];
>>
>> pdf_uuid_t t = pdf_uuid_generate (PDF_UUID_TIME);
>>
>> printf("Generated UUID, time 1: %s\n", pdf_uuid_string (t, t_a));
>>
>
> You shouldn't do that. The idea to pass the array specifying its size is
> in order to tell the developer the size of the array that needs to be
> passed. So, do not try to pass a 1-byte array or a non-allocated array,
> when you're kindly asked to pass a PDF_UUID_CHAR_LENGTH-byte array :-)
> And when you pass the array like that, you're just passing it as "the
> address of the first byte of the buffer".
>
> If you pass a 1-byte, and the function then writes PDF_UUID_CHAR_LENGTH
> bytes, you'll be writing out of buffer and corrupting the stack (at
> least in this case with the buffer in the stack).
>
> If the buffer is not allocated at all and you pass it, you'll be passing
> a pointer to nowhere known, and that will definitely crash
> instantaneously.
>
> Spend some time running your test under valgrind and you'll see it
> complaining about invalid reads and writes.
>
>>
>> So, regarding the first case, is gcc re-allocating the char array when
>> it reads the function header?
>
> No, there's no gcc magic involved here. The array needs to be fully
> allocated either in stack or heap and have PDF_UUID_CHAR_LENGTH bytes.
> If the programmer doesn't do so, it's a programmer error.
>
> Either way, what Jose said about using:
>  void pdf_uuid_string (pdf_uuid_t, char * ascii_rep);
> is equally fine if we specify in the documentation that the ascii_rep
> array needs to be preallocated and have at least PDF_UUID_CHAR_LENGTH
> bytes.
>
> Cheers,
>
> --
> Aleksander
>

patch-amp-2011-03-03
Description: Binary data