Re: [PATCH 2/3] sdp: Limit side effects of sdp_get_data_type and sdp_get_data_size

[Bart Westgeest <bart@elbrys.com>]

Hi Anderson,

>> -       sdp_get_data_type(buf, d->dtd);
>> -       sdp_get_data_size(buf, d);
>> +       buf->buf_size += sdp_get_data_type_size(d->dtd);
>> +       buf->buf_size += sdp_get_data_size(buf, d);
>
> No need to check for "if (!buf->data)" like in the original code?

I do not think there's a need for it, this modification is in 
sdp_gen_buffer. sdp_gen_buffer is called three times from the following 
functions: (1) sdp_attr_size, (2) sdp_append_to_pdu, and (3) 
gen_dataseq_pdu.

For (2) & (3), buf is memset to 0 prior to calling sdp_gen_buffer, thus 
buf->data is always NULL.

For (1), buf is also memset to 0 for the first call to sdp_attr_size, 
which is called repetitively in a foreach statement. Repetitive calls to 
sdp_gen_buffer on the same buffer will not (ever) result in an 
allocation of buf->data as far as I can tell.

>> -       pdu_size = sdp_get_data_type(buf, dtd);
>> +       pdu_size = sdp_get_data_type_size(dtd);
>
> Same here. Additionally, buf->buf_size is not being updated.

This modification is in sdp_gen_pdu. sdp_gen_pdu is never called with 
buf->data == NULL. If it would, then the initialization of seqp in 
sdp_gen_pdu is erroneous, since it uses buf->data without verifying it.

In addition, in the current code, sdp_gen_pdu is only called from three 
locations: (1) get_data_size, (2) sdp_append_to_pdu, (3) 
gen_dataseq_pdu. In all cases the buf->data is either allocated prior to 
calling (2 & 3), or (1) there's an if statement preventing sdp_gen_pdu 
from being called when buf->data == NULL.

Bart