Re: [PATCH] [BUILTIN] Allow SIG* signal names.

[Isaac Jurado <diptongo@xxxxxxxxx>]

On Mon, Jul 2, 2012 at 9:07 PM, Eric Blake <eblake@xxxxxxxxxx> wrote:
>
> I'm not the maintainer, so my decision is not indicative of what the
> dash maintainer will choose.  But my personal preference would be that
> we change this area of code, either to:
>
> 1. be lighter-weight (drop strcasecmp, which is locale-dependent, and
>    replace it with strcmp)
>
> 2. be more user-friendly (accept optional case-insensitive SIG prefix)
>
> Both approaches are permitted by POSIX, so it boils down to a judgment
> call of whether providing useful extensions or providing a minimally
> compliant shell is more important.

Reasonable enough.

> Even one byte larger in the final executable size has been deemed
> bloat on this list in the past.  Dash prides itself on being
> minimalistic, but you happened to point out an area of code that is
> not currently minimal.

But swapping strcasecm with strcmp is not going to affect final binary
size (well, except for the extra bytes in the PLT), so I deduce that
bloat for dash also means runtime efficiency.

> Indeed, that would provide faster lookup, but it would also cost more
> executable size (the storage requirements for a perfect hash table are
> larger than the size of a loop comparison); I don't know whether the
> preference is for speed, for minimal size, or for a hybrid of the two
> (where larger size is okay only if it proves to have more speed).  So
> hopefully the dash maintainer will chime in on the topic.

I'm looking forward.  Thanks a lot for the explanations.

Cheers

P.S.: I guess the SIG* prefix check may be "minimized" with some ugly
casts and masks (in a platform dependent way and assuming the string is
properly aligned, of course).

-- 
Isaac Jurado

"The noblest pleasure is the joy of understanding"
Leonardo da Vinci
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html