On Thu, Nov 12, 2015 at 7:22 PM, Dan Cross <[email protected]> wrote:

> On Thu, Nov 12, 2015 at 3:02 PM, Barret Rhoden <[email protected]>
> wrote:
>
>> > +static void kprof_usage_fail(void)
>> > +{
>> > +     static const char *ctlstring = "clear|start|stop|timer";
>> > +     const char* const * cmds = profiler_configure_cmds();
>> > +     size_t i, size, csize;
>> > +     char msgbuf[128];
>> > +
>> > +     strncpy(msgbuf, ctlstring, sizeof(msgbuf));
>>
>
> This is arguably incorrect. If one decreased the size of 'msgbuf' or made
> ctlstring larger so that sizeof(msgbuf) <= strlen(ctlstring), this this
> would omit the trailing NUL on 'msgbuf'. Even if that's not true, this is
> overwriting every byte of 'msgbuf' beyond the the copy of 'ctlstring' with
> 0s, seemingly unnecessarily.
>

Right that could be strlcpy(). Will change that.
But, on one side you are worried about the performance (which is a function
like this, arguably means nothing) of filling ~100 zeros, and ...


> +     for (i = 0; cmds[i]; i++) {
>> > +             csize = strlen(cmds[i]);
>> > +             if ((csize + size + 2) > sizeof(msgbuf))
>> > +                     break;
>> > +             msgbuf[size] = '|';
>> > +             memcpy(msgbuf + size + 1, cmds[i], csize);
>> > +             size += csize + 1;
>> > +     }
>> > +     msgbuf[size] = 0;
>>
>
> This loop could be replaced with,
>
>   strlcpy(msgbuf, ctlstring, sizeof(msgbuf));
>   for (int i = 0; i < nelem(cmds); i++) {
>     strlcat(msgbuf, "|", sizeof(msgbuf));
>     strlcat(msgbuf, cmds[i], sizeof(msgbuf));
>   }
>

... here, you are suggesting to walk the string from the first byte using
strlcat() ☺

-- 
You received this message because you are subscribed to the Google Groups 
"Akaros" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to