On Fri, Nov 13, 2015 at 8:34 AM, 'Davide Libenzi' via Akaros < [email protected]> wrote:
> I argue that strcat() as interface is bad enough done once, but done in a > loop, by continuously throwing away the buffer length at every call, is > even evil. > Anyway, I chose evil in this case, as that function could be written in > Cobol and still be performant enough, but that strcat loop should not be > taught at school 😀 > Being evil is good. It means you can jump over buses on a bike while blasting slayer in the background: https://www.youtube.com/watch?v=WuHpP10YF7M&t=10 > > > On Thu, Nov 12, 2015 at 7:41 PM, Dan Cross <[email protected]> wrote: > >> On Thu, Nov 12, 2015 at 10:33 PM, 'Davide Libenzi' via Akaros < >> [email protected]> wrote: >> >>> 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() ☺ >>> >> >> I don't care about the performance; I just don't see the point of zeroing >> the rest of the buffer. >> >> >>> >>> >>> -- >>> 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. >>> >> >> -- >> 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. >> > > -- > 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. > -- 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.
