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.