From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Garzik Subject: Re: [Patch] libcldc,cldcli: Use humanized error messages Date: Wed, 26 Aug 2009 23:04:52 -0400 Message-ID: <4A95F7D4.4000805@garzik.org> References: <20090826200625.4434935f@redhat.com> <4A95EEEA.9000301@garzik.org> <20090826205003.79e4b2af@redhat.com> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20090826205003.79e4b2af@redhat.com> Sender: hail-devel-owner@vger.kernel.org List-ID: Content-Type: text/plain; charset="us-ascii"; format="flowed" To: Pete Zaitcev Cc: Project Hail List On 08/26/2009 10:50 PM, Pete Zaitcev wrote: > On Wed, 26 Aug 2009 22:26:50 -0400, Jeff Garzik wrote: >> On 08/26/2009 10:06 PM, Pete Zaitcev wrote: > >>> Signed-off-by: Pete Zaitcev >> >> applied a modified version, see attached... after that short IRC >> discussion, I think an array with a range check would be nicer than >> coding it (though the C compiler should make a nice, efficient table >> from 'switch' statement for us...) > > I don't think it's enough. Look: > > +static const char *cld_errlist[] = > +{ > + [CLE_OK] = "Success", > + [CLE_SESS_EXISTS] = "Session exists", > > When you do indexed initializations, it's possible to create > empty array elements (currently there are none because indexes > are contiguous). So, the following needs a NULL check: > > + return cld_errlist[ecode]; > +} True in general, but do you actually see a hole? It seems to me that our error list will always be sequential, without holes. Even if we obsolete an error, we still need to keep it around in the headers and cld_errlist[]. And new error codes will go on the end of the list... Jeff