Re: [PATCH PR/42686] Align the help text output

["Joseph S. Myers" <joseph@codesourcery.com>]

On Wed, 10 Mar 2010, Shujing Zhao wrote:

> +/* Expand the ROOM when the number of bytes of MSGSTR is larger than
> +   the width in columns. */
> +
> +unsigned int
> +get_col_width (const char *msgstr, unsigned int room)

The comment on this function makes no sense to me at all.  What does 
"Expand the ROOM" mean?  It sounds as if ROOM is a parameter passed by 
reference that might be changed by the function - but that clearly isn't 
the case for a parameter of type unsigned int.  What are the semantics of 
the operand ROOM?  What are the semantics of the return value?  (But see 
below on how "the number of bytes of MSGSTR is larger than the width in 
columns" is itself not a sensible concept for the code to be considering 
in the first place.)

> +      if (screen_cols != nbytes)

This comparison is conceptually confused.  In logical terms, "columns" and 
"bytes" are different data types that it does not make sense to compare.  
It so happens that in the present C code they have the same static type so 
the host compiler doesn't give an error for this comparison - but it is 
still confused, and you need to arrange the code so it doesn't try to mix 
different types like this.  This means having a very clear notion of 
whether a particular variable counts columns, bytes or characters, and of 
how you do explicit computations of the number of each kind of object in a 
particular part of a string, and no comparisons between counts of 
different things.

-- 
Joseph S. Myers
joseph@codesourcery.com