Re: [PATCH 1/3] stkutil: display text attributes as html

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1681 bytes --]

Hi Kristen,

On 07/01/2010 01:29 PM, Kristen Carlson Accardi wrote:
> On Thu, 01 Jul 2010 11:30:07 -0500
> Denis Kenzior <denkenz@gmail.com> wrote:
> 
>>
>>> +		start = attrs[i++];
>>> +		len = attrs[i++];
>>> +		code = attrs[i++];
>>
>> You might want to be extra paranoid here that attrs_len is a multiple of 4.
> 
> attrs_len does not have to be a multiple of 4.  I will add a sanity
> check to attrs_len to make sure it is at least >= i + 3.
> 

Then this code is incorrect, as it handles len 3 attrs only at the end
of the array.  Also please note that SIM Toolkit Text Attributes are
always coded on 4 bytes (see 102.223 Section 8.72 for details).  You
might want to assume 4 byte alignment or invent a data structure for
these attributes.

>>
>>> +
>>> +		if (i < attrs_len)
>>> +			color = attrs[i++];
>>> +		else
>>> +			color = 0;
>>> +
>>> +		if (len == 0)
>>> +			end = text_len;
>>> +		else
>>> +			end = start + len;
>>> +
>>> +		/* sanity check values */
>>> +		if (start > end || end > text_len)
>>> +			continue;
>>> +
>>> +		/*
>>> +		 * if the alignment is the same as either the default
>>> +		 * or the last alignment used, don't set any alignment
>>> +		 * value.
>>> +		 */
>>> +		if (start == 0)
>>> +			align = STK_DEFAULT_TEXT_ALIGNMENT;
>>
>> Are attributes which do not contain start = 0 valid?  If so, you might
>> take extra care here.
> 
> Yes start can be nonzero, but I don't understand what you are asking here.
> If start is not equal to zero, we have already made sure it is not
> greater than the end.

I'm not seeing how align gets initialized properly if start != 0.

Regards,
-Denis