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

[Kristen Carlson Accardi]

On Thu, 01 Jul 2010 13:54:16 -0500
Denis Kenzior <denkenz at gmail.com> wrote:

> 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 at 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.

I should have said that attrs_len does not always have to be a multiple
of 4 -- it is allowed to only have 3 byte attrs as the last attribute,
so the code is correct.  However, since not all attributes are
required to be of len 4, we can't just assume attrs_len is a multiple
of 4.

> 
> >>
> >>> +
> >>> +		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.

I snipped that part off of my reply, sorry.  Here is the code as I
originally sent it:
 		/*
                 * 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;
                else {
                        align = (formats[start -1] & 0xFF) &
                                        STK_TEXT_FORMAT_ALIGN_MASK;
                        if (align == STK_TEXT_FORMAT_NO_ALIGN)
                                align = STK_DEFAULT_TEXT_ALIGNMENT;
                }

so if start != zero, align is set to the previous alignment value.
(formats[start - 1]).

Thanks,
Kristen