Code review comment for lp:~mm-yuhu/gearmand/server-funcs

Revision history for this message
Eric Day (eday) wrote :

The language/charset issues with strcasecmp are irrelevant here because
it's only used for predefined server commands, nothing that is user
defined. Since strcasecmp is used in code immediately surrounding it
now, it should also be used here for consistency. If there is reason
to remove the use of it, that should be done everywhere, probably in
a different branch.

-Eric

On Thu, Mar 11, 2010 at 06:54:15PM -0000, Brian Aker wrote:
> Hi!
>
> On Mar 11, 2010, at 10:36 AM, HackMan wrote:
>
> > I used strcasecmp since this is how the status command was
> > implemented. If you want I can rewrite it using a combination of to
> > lower and strcmp.
>
> The bigger issue is that if someone tosses a UTF-8 string in, we will
> incorrectly lowercase it (which... we aren't perfect on this already).
>
> The test case is more important :)
>
> Cheers,
> -Brian
> --
> https://code.launchpad.net/~mm-yuhu/gearmand/server-funcs/+merge/21142
> Your team Gearman-developers is subscribed to branch lp:gearmand.

« Back to merge proposal