• Debra (unregistered)

    strdup() isn't standard C.

  • LCrawford (unregistered)

    This has so many WTFs embedded. Has all the signatures of someone struggling and copying code fragments from everywhere until the output looked correct.

    They probably did try the ' return "true"' pattern, but the compiler complained because it returns char * instead of const char *. So they reverted to the memory leak pattern.

  • (nodebb)

    The __ at the top leads me to believe this is their convention for making the function "private"

    That's as may be, but it also makes the code inherently broken, since double-underscores at the beginning of a C identifier (and anywhere else in C++) classes the identifier as "reserved for the system", which this code is surely not part of.

  • Hanzito (unregistered)

    The reason might be that they will (may?) call free() on the result.

  • (nodebb)

    The only advantage to their strcat version would be returning a non-const (aka mutable) pointer. Returning the const char* directly would give an immutable version. However, returning a mutable buffer seems pointless as it's allocated to fit the exact size of the returned string, meaning any operations on it must copy it to a larger buffer already...

  • Pyth0n (unregistered)

    Not long ago I helped to fix a code that was using strtok() on a string literal (yes, a homework). Kernel has been killing it with not very helpful message (at least for a beginner)... For those not familiar: strtok()modifies in-place the string being parsed, to avoid copying. Trying to modify .rodata/.text memory areas (where string literals reside) is a no-no.

    Indeed, const * char is a hairy stuff, especially when the consumer mistreats it.

  • rkapl (unregistered)

    The code given is not equivalent in practice. In the original example you must call free on the result, in the second one you do not have to and cannot. But you could do return strdup("true")

  • Industrial Automation Engineer (unregistered)

    ... but given that this function's job is ...

    this is TRWTF. never, never, never make any assumptions of the context / environment of your function, or give instructions on how it /should/ be (properly) used.

  • Industrial Automation Engineer (unregistered)

    ... but given that this function's job is ...

    this is TRWTF. never, never, never make any assumptions of the context / environment of your function, or give instructions on how it /should/ be (properly) used.

  • (nodebb) in reply to Steve_The_Cynic

    "reserved for the system", which this code is surely not part of

    I've seen plenty of system code that was little better than this.

  • (nodebb) 
    But all of that is completely unnecessary, because you can just do this:

char* __comps_num2boolstr(COMPS_Object* obj) {
    if (((COMPS_Num*)obj)->val) {
        return "true"
    } else {
        return "false"
    }
}

    Except there still remains a WTF in that corrected code. You're returning const strings as non-const theoretically allowing them to be modified, and thus invoking undefined behaviour. (Of course, any reasonably configured compiler would be pointing this out at compile time...)

  • Yoh (unregistered) in reply to PJH

    Exactly - came here to say this. And this is also as good a case for a ternary as I've ever seen:

    const char* theRealWTFIsTheFunctionName(COMPS_Object* obj) { return ((COMPS_Num*)obj)->val ? "true" : "false"; }

  • (author) in reply to PJH

    "Warnings are an exercise for the reader," should be assumed any time I post my own version of the code.

  • RLB (unregistered) in reply to Steve_The_Cynic

    That's as may be, but it also makes the code inherently broken, since double-underscores at the beginning of a C identifier (and anywhere else in C++) classes the identifier as "reserved for the system", which this code is surely not part of.

    Also, it doesn't even work. What works is making the function static.

  • (nodebb)

    You can bet the calling code doesn't call free() on the returned string, resulting in a memory leak... memory that should never have been allocated in the first place. I haven't coded in C (or any other low-level language) for 15 years, and I'm still shocked at how bad that code is.

  • TheCPUWizard (unregistered)

    char *c = __comps_num2boolstr(Cobj); free(c);

    Yeah, that will go over REAL good with the "improvement" suggested.......

    (also, this is quite possibly from a calculation engine that uses tables of function pointers that have a consistent signatue....

  • (nodebb) in reply to tom103

    you can bet the calling code doesn't call free() on the returned string

    Actually, I would bet against that. I've seen this pattern used before precisely so you can treat the return value the same as a dynamically allocated string and therefore not worry about whether you can free it or not.

  • (nodebb) in reply to jeremypnet

    Yeah, not much of a WTF to me, for this reason. Also, strdup() is a POSIX function, not a standard C one. We don't know about their conformance requirements, they could be targeting old and/or embedded systems.

    Which leaves the naming convention as WTF, and not much else.

    Addendum 2023-05-24 11:33: Ah, looks like strdup() is now a standard C function... since C23, which means it likely won't compile without warning in Visual Studio before 2035.

  • (nodebb) in reply to Steve_The_Cynic

    To be fair, this is an convention not an enforced rule. But definitely bad practice for sure.

  • Worf (unregistered)
    Comment held for moderation.
  • Erik (unregistered) in reply to Debra
    Comment held for moderation.
  • fzsz (unregistered)
    Comment held for moderation.
  • Shivani Desai (unregistered)
    Comment held for moderation.
  • C man (unregistered) in reply to Steve_The_Cynic

    This is wrong, that is just a clang convention

  • C man (unregistered) in reply to Steve_The_Cynic

    This is wrong, that is just a clang convention

  • (nodebb) in reply to C man

    No. Just no.

    It long predates even the existence of clang, and is enshrined in C as early as the C89/90 standards.

Leave a comment on “Truly Strung Out”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article