Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

I've also grown somewhat sensitive to duplication, maybe to a painful level. But, the memmove-call from the AI writeup has duplication in there:

    memmove(args->begin_argv + extend,
            args->begin_argv + consume,
            args->endp - args->begin_argv + consume);   // ← bug
If both `args->begin_argv + consume` are supposed to be the same concept and thus the same value, I'd have a variable for it by now. Some people hate it with a passion, but something like this removes the precendence thinking, prevents modification of one and not the other and makes it easier to follow, for me at least:

    retained_tail_begin = args->begin_argv + consume
    memmove(args->begin_argv + extend,
            retained_tail_begin,
            args->endp - retained_tail_begin);
Though at that point one might also encode the entire intent (as far as I understand it) in variables as well:

    space_to_replace_end = args->begin_argv + extend
    retained_tail_begin = args->begin_argv + consume
    memmove(space_to_replace_end,
            retained_tail_begin,
            args->endp - retained_tail_begin);
Sure we can golf the names somewhat, but that code has my head spin a lot less about math and precedence.


Yeah - sharing a variable there is a pretty strong signal that they are the same concept and can't be allowed to be different, not just "maybe the same value". That's useful to know.

If there's a ton of it in a dense bit of code, 1) it might be too complex, try making it clearer, and 2) it unfortunately makes for a lot of indirection and that can make it harder to follow, which is generally why I see people dislike it. In non-critical code I can kinda agree with inlining it. Pointer arithmetic is imo never non-trivial tho, paranoia is warranted. Especially in a kernel.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: