• (nodebb)

    Even the requirements don't make sense to me. What's the point of taking a large count of objects and byte copying them to a huge memory buffer? What could possibly be the benefit of that?

  • Labasta (unregistered)

    TRWTF is writing in C. Of any flavour.

  • Debra (unregistered)

    The code, as posted, memcpy's the whole ImportantThing structure, which means that the pointer to "details" is copied too. Assuming that this is actual serialization for I/O or otherwise transferring data to other computers, it makes absolutely no sense to do that. It's just wasted bytes.

  • (nodebb) in reply to Debra

    Yes, but compared to the bug in the call to memcpy, what you mention there is irrelevant.

    memcpy(positionInBuffer, &arrayOfThings[i].details, stringLen);

    It memcpy()s the pointer and whatever follows it into the output buffer, not the string.

    Addendum 2023-06-19 07:47: (note the & in front of the second parameter)

  • Pabz (unregistered)

    It looks like the Very Smart and Very Senior developers made a lot more assumptions than just within their code. They blamed Stefano for the problem then seemingly left him on his own to fix it without actually telling him what he'd done wrong. If I'd been a junior developer on that team and had been blamed for something I'd have asked immediately what I'd done wrong as a learning experience.

    As things stand I'm actually an experienced developer. If I think someone else on my team has made a mistake then I will confirm it and be prepared to fully explain it to them. Also I am quite prepared for other members of my team to suggest improvements to my code and point out any mistakes I've made, including the juniors. Juniors make valuable contributions as well of course, otherwise they wouldn't be working for the company.

  • (author) in reply to Steve_The_Cynic

    I'm gonna guess that was a mistake in the submission that didn't exist in the original code.

  • Worf (unregistered)
    Comment held for moderation.
  • (nodebb) in reply to Remy Porter

    @Debra, Steve_The_Cynic and Remy Porter

    memcpy(positionInBuffer, &arrayOfThings[i].details, stringLen); I'm gonna guess that was a mistake in the submission that didn't exist in the original code

    I don't have access to this codebase any longer: like the previous time, I tried to reconstruct it from memory (carbon-based memcpy???), so I often make mistakes: in this case, the ampersand is my fault. I apologize :(

    Instead, the WTF in this line

    memcpy(positionInBuffer, &arrayOfThings[i], sizeof(ImportantThing)); was really in the original code. YES, it also copied the "details" pointer. YES, it made no sense. Anyway, it worked: why? Because the serialized pointer was NOT used as a pointer! During deserialization (not shown in the snippet, because my eyes and fingers were already bleeding after reconstructing the serialization code) it acted as a kind of boolean value: if it was NULL, the item had no extra string and you could move to the next item; if it was not-NULL, there WAS an extra string that had to be deserialized before moving to the next item. The "pointer" value was then discarded and replaced with a freshly allocated one, a real one. YES, it was a horrible kludge. Not the first one in that application, and unfortunately not the last one.

    Addendum 2023-06-20 02:14: Sorry for the bad formatting (a Preview button would be useful)

  • (nodebb) in reply to Mr. TA

    @Mr. TA

    Even the requirements don't make sense to me. What's the point of taking a large count of objects and byte copying them to a huge memory buffer? What could possibly be the benefit of that?

    Even if the application is 20-years old, I don't want to disclose too much...

    Anyway, it had to do with the serialization functions shown in my previous WTF (Remy linked it in the post). The single buffer (usually small, but not this time) was typically sent (probably via socket) from a client application to a server application as a request, then the server replied with another buffer.

    The "benefit" was that each request-response was a single round-trip. Funny, because IIRC both the client and server were running on the same computer!

    @Labasta

    TRWTF is writing in C. Of any flavour

    I agree NOW, but 20 years ago it seemed a reasonable choice (also considering the huge mess the previous application was: no, I won't disclose details, I already said too much).

    Generally speaking, the performance DID improve, except for this awful part I posted. Of course, it had its own share of problems :(

    Since I can't time travel, at least this site gives me the chance to vent it out :D

  • Mike5 (unregistered) in reply to Pabz
    Comment held for moderation.
  • (nodebb) in reply to StefanoZ

    That was exactly my thought, actually - the pointer has to be serialized so that it can be used as a boolean. Thanks for confirming that to all of us.

  • (nodebb) in reply to StefanoZ

    Sure, but why not write directly to the socket? Why create an intermediate buffer at all?

  • (nodebb) in reply to StefanoZ

    OK, thanks for explaining about the &.

  • Steven (unregistered)
    Comment held for moderation.
  • (nodebb) in reply to Mr. TA

    Sure, but why not write directly to the socket? Why create an intermediate buffer at all?

    At last I unearthed an old copy of the source code (MY EYES! MY EYEEEEEES!!! well, the bright side is: maybe I'll be able to dig out other juicy, time-barred WTFs...)

    Do you remember the Ancient Library with the APIs such as "TheLibraryName_Client_SerializeInt", "TheLibraryName_Client_SerializeDouble", etc from the previous WTF?

    There was also an API like this:

    int TheLibraryName_Client_SendMessage(BLAHBLAH_CONNECTION* connection, char* requestBuffer, char replyBuffer, int replyBufferSize, / blah blah other arguments */);

    it was supposed to hide the actual way the communication was handled (There were equivalent APIs to process messages from the server side and send the reply). It PROBABLY hid a socket, but I cannot know for sure, because I found only the source code of the application, not the one of the library.

    (Imagine a time portal opens and someone from today tells my colleagues of 20 years ago: "What about some nice REST via HTTP? Or better HTTPS?". They would say "WTF?" and attack with torches and pitchforks...)

    Addendum 2023-06-20 14:19: Whoops the comment system interpreted the asterisks as formatting. As you can imagine, "replyBuffer" requires two stars and "replyBufferSize" one star, they are output parameters. And after that, a slash-star comment begins.

  • I'm not a robot (unregistered) in reply to StefanoZ
    Comment held for moderation.
  • alwaysviral (unregistered)
    Comment held for moderation.
  • alwaysviral (unregistered)
    Comment held for moderation.

Leave a comment on “Assumption is the Mother of all Segfaults”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article