<URL: http://bugs.freeciv.org/Ticket/Display.html?id=40028 >

William Allen Simpson wrote:
> <URL: http://bugs.freeciv.org/Ticket/Display.html?id=40028 >
> 
> Jason Short wrote:
>> Here is a quick and partial fix.  I assume that strerror() is one of the
>> most common offending functions, so I quickly went through and converted
>> all mystrerror users in client/ and server/ directories to use the
>> newly-written L_(). 
> 
> Now, I find this irritating on a number of levels.
> 
> 1) This ticket was already patched and resolved.  Another new ticket would
> be better.  I'll open one.

What would possibly motivate you to resolve a bug that's still unfixed, 
mere hours after making a rant about how writing off bugs as acceptable 
is sloppy bugfixing?

> 2) Madeline reported the problem was in mystrerror(), so that's a good
> target,

Ahh yes, I see that now.  Obviously I had only remembered it 
subconsciously before :).  Speaking of which, Madeline, what is the 
mystrsocketerror function you mention?

> but the solution would be to patch mystrerror(), not repeatedly
> patch every invocation....  This is *all* users!
> 
> 3) By definition, mystrerror() can only be called once, so there's no
> technical reason to use a dynamic buffer.

Yes.  Naturally the first thing I considered was making the change 
directly within mystrerror, as that is far far easier. But, if one 
mystrerror call is done before the previous one's return value has been 
discarded the result will be erroneous.  Given that some of the returned 
strings are passed off into arbitrary callbacks this is a rather risky 
thing to assume IMO.  However the error if this happened would be 
non-fatal so I suppose if we want to accept that bug it's okay.

> 4) I really hate L_() hiding a simple single function.  There's no excuse.
> This makes the code harder to read and harder to grep.
> 
> 5) The parameter order doesn't match either cat_snprintf() or strlcat(),
> or any standard C invocation.  By convention, the destination buffer is
> always the first parameter.

It aims at consistency with _, N_, PL_, and Q_, with which it shares 
similar naming.  All of these are just shortened wrappers for single 
functions that allow easy and common use, and all put the text parameter 
first.  Of course L_ does no translation, it just changes encoding (as 
do the other _ functions).

-jason



_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

Reply via email to