From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9813 invoked by alias); 10 Mar 2010 17:05:32 -0000 Received: (qmail 9528 invoked by uid 22791); 10 Mar 2010 17:05:28 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_FAIL X-Spam-Check-By: sourceware.org Received: from mx20.gnu.org (HELO mx20.gnu.org) (199.232.41.8) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 10 Mar 2010 17:05:24 +0000 Received: from mail.codesourcery.com ([38.113.113.100]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NpPLV-0008RF-Ro for gcc-patches@gcc.gnu.org; Wed, 10 Mar 2010 12:05:22 -0500 Received: (qmail 14578 invoked from network); 10 Mar 2010 17:05:18 -0000 Received: from unknown (HELO digraph.polyomino.org.uk) (joseph@127.0.0.2) by mail.codesourcery.com with ESMTPA; 10 Mar 2010 17:05:18 -0000 Received: from jsm28 (helo=localhost) by digraph.polyomino.org.uk with local-esmtp (Exim 4.69) (envelope-from ) id 1NpPLQ-0003z8-O3; Wed, 10 Mar 2010 17:05:16 +0000 Date: Wed, 10 Mar 2010 17:19:00 -0000 From: "Joseph S. Myers" To: Shujing Zhao cc: GCC Patches , Paolo Carlini Subject: Re: [PATCH PR/42686] Align the help text output In-Reply-To: <4B976D51.8010705@oracle.com> Message-ID: References: <4B863559.4000407@oracle.com> <4B976D51.8010705@oracle.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-detected-operating-system: by mx20.gnu.org: GNU/Linux 2.6, seldom 2.4 (older, 4) Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2010-03/txt/msg00402.txt.bz2 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