Magical C++ NULL



  • I'm starting to suspect the only way to get anywhere in this industry is to be utterly incompetent.


    Case in point: Staff level engineer, which is above Senior and about as close to an executive level engineer as you can get without putting a hit out on someone. Anyway, he had a code review for a random crash during the cleanup of a C++ class. One of his fixes resulted in him deleting an object in memory, then not setting the class variable to NULL afterwards. Since it's possible for later calls to try and refer to this object to see if it's valid or not, it needed to be set to NULL.


    So after he was called on it, he checked in some code that set the pointer to NULL then called delete on it.


    When he got called on that he removed the delete call, as it's 'superfluous'. I've asked him to explain, and am looking forward to the reply.


    And before you ask, no, he's worse at C# and Managed C. It's frightening that anyone with his supposed 20 years of experience thinks that the correct way to hide text on a UI is to set the text to the same color as the background of the window, or their method of 'disabling' a pulldown menu is to stick a disabled text box over it.


    Yet he makes probably twice what I do, and gets to leave every day at 4. In fact, I've never seen him here after 4, even during crunch periods, and never on weekends. Not that I'm complaining, we get more work done when he's gone, but I know he'll get a bonus this year and the rest of us won't, because he's "senior or above".


    Why no, I'm not bitter. Not at all.



  • Obviously the delete call was superfluous: deleting a NULL pointer does nothing. 😉



  •  Obviously he's forward-thinking and will be adding multithreading. Have a dedicated "deleter" thread, set the pointer to NULL in one, simultaneously instruct the deleter to read the pointer value and delete the object in the other and the delete will happen before the core on which you set the pointer to NULL has had time to flush its L1-cache to memory, so the deleter thread reads the old value and deletes that.


  • Winner of the 2016 Presidential Election

    Because that would be less WTFy.



  • @Evo said:

    Obviously the delete call was superfluous: deleting a NULL pointer does nothing. 😉



    Ok, I'll give you that. 🙂



  • @CodeNinja said:

    Yet he makes probably twice what I do, and gets to leave every day at 4. In fact, I've never seen him here after 4, even during crunch periods, and never on weekends. Not that I'm complaining, we get more work done when he's gone, but I know he'll get a bonus this year and the rest of us won't, because he's "senior or above".

    I've had one colleague who was a demonstrably, objectively terrible programmer and was getting paid 10K more than most of the rest of the team. Whenever a project was fucked up, he always seemed to be involved and always had a million reasons why it was management's fault. His code was awful to read and he had an unusually extensive level of experience in the use of his keyboard's Ctrl, C and V buttons.

    When the Bad Times rolled around, he was first on the chopping block. Sometimes, in my darker moments, I admit to myself that he was only made redundant because of his higher pay, and that management were completely oblivious to what a shitty colleague and programmer he was. This wasn't even that big of a company so I can only imagine how easy it must be for this sort of poisonous person to hide under the radar in BigCorp.

    Sometimes I ask myself how such a person manages to elevate themselves to such a high level of remuneration. The answer, I believe, may be as simple as "By asking for more money a lot".



  •  @CodeNinja said:

    @Evo said:
    Obviously the delete call was superfluous: deleting a NULL pointer does nothing. 😉
    Ok, I'll give you that. 🙂
    On a decent system, deleting NULL will crash your program, and if you're writing a driver, the whole system, just the way God has intended.

    Anyway, your pain is understood. I've seen someone start making promotion by being a loud-mouthed know-it-all in meetings. His work was ok, but he wasn't really technically very savvy or anything. Yet his attitude, the fake cameradery with one of the veeps, etc., made him look good in the eyes of management, whose main frame of reference is exactly that behaviour...



  • @TGV said:

    @CodeNinja said:
    @Evo said:
    Obviously the delete call was
    superfluous: deleting a NULL pointer does nothing. 😉
    Ok, I'll give you
    that. 🙂
    On a decent system, deleting NULL will crash your program, and
    if you're writing a driver, the whole system, just the way God has intended.
    Um, no. In C++ deleting a null pointer is a no-op (as is the case in C with free().) What is less forgivable is that this behaviour is the exception, rather than the rule. fclose(0) or close(-1) are undefined, for example.



  • @GNU Pepper said:

    Sometimes I ask myself how such a person manages to elevate themselves to
    such a high level of remuneration. The answer, I believe, may be as simple as
    "By asking for more money a lot".

    Or it's the Dilbert principle at work - though in your case it seems to have failed in the fact it didn't take him out of the workflow.



  • Deleting NULL crashing your program would be....a broken system. Deleting a NULL pointer is legit C++ spec.



  • @PJH said:

    In C++ deleting a null pointer is a no-op (as is the case in C with free().) What is less forgivable is that this behaviour is the exception, rather than the rule. fclose(0) or close(-1) are undefined, for example.


    I thought it was something like this, honestly I've not deleted a NULL pointer in so long that I don't really remember what happens. It's just become habit for me to check the pointer for NULL before deleting it, if it can be deleted in more than one location, or I suspect someone else may modify the code later.


    Oh, goody, he finally replied to the review!

    "If we set m_pSettings to NULL first, the freeing code for MDelete isn't executed because it checks to see if the pointer passed to it is not null. See Line 232 (DeleteFree) in MemoryManager.h."



    I think he's missing the point. What's funnier is that the Project Lead, another Senior Level Engineer (great guy!), and myself are all telling him to just move his "m_pSettings = NULL;" down one line so it occurs after the delete call, and he's arguing it.


    His 'fixes' also haven't solved the problem, but that's not a surprise to anyone. I'm half tempted to come in this weekend and fix the issue for him, after re-assigning the task to myself in JIRA, but I'm so far behind on my own stuff that I probably shouldn't.



  • @CodeNinja said:

    I think he's missing the point.
    He is. It's a classic memory leak. Are you able to use, for example, valgrind against the application concerned?



  • @PJH said:

    @CodeNinja said:
    I think he's missing the point.
    He is. It's a classic memory leak. Are you able to use, for example, valgrind against the application concerned?


    At the moment, no, and now that you've mentioned it I'm a little concerned since he's been put in charge of finding software for that kind of profiling. Honestly, though, I'm not horribly surprised since people who've worked with him previously on other teams called him the 'memory leak king'. Part of me wants to turn on the leak detecting functionality in the memory manager he's working on, to see if it's able to capture a leak within itself.


    I should back up a little, though. Reading through some of the comments it seems like some of you believe that his comment about removing the delete fixed the crash. This isn't the case, in fact we're not really sure why he was messing with this function, as the crash actually happens in a completely different function that's sole task is to delete another pointer. He flat out just removed that delete, so that pointer isn't being deleted either, and it's pointing to a fairly large object as well. According to his comments, the reason he deleted it was because, "things were happening out of order and the pointer was invalid with a value of 0xddddddddd". After talking with a coworker about my suspicions, we're fairly certain that's a Microsoft static value that gets set when a pointer is deleted but not NULLed. It appears that the delete is being called twice, which it would be able to handle if someone just wrapped it in an 'if' test and set it to NULL after deleting. It's kind of amusing, in a stressful way, to watch the Team Lead and another Senior Engineer try to lead him to this solution, he's actually fighting back.



  • This type of WTF actually makes me appreciate the review process where I work, as painful as it is.



  • @GNU Pepper said:

    Sometimes I ask myself how such a person manages to elevate themselves to such a high level of remuneration. The answer, I believe, may be as simple as "By asking for more money a lot".

     

     Yes. Raises are decided mainly by human contact with the HR departement usually, and even if a superioris supposed to give his technical opinion, je may not directly read his code for lack of time, of simply have a good opinion of him and zsuppose that if he is sympathic he is competent.

    Once someone is senior enough, he is much less likely to getcalled on error on top of that, because people who have elevated him in the first place think they had good reason to do so. The sad thing : if you were in the shoes of the HR people or superior, you would be very likely to be tricked too. Human are wired to favor their friend, not the competent people.

     



  • @GNU Pepper said:

    Sometimes I ask myself how such a person manages to elevate themselves to such a high level of remuneration. The answer, I believe, may be as simple as "By asking for more money a lot".

     

    It's called "being a sub-criminal psychopath".  It's symptoms are rigorous suppression of anyone below you, and diligent attendance on anyone above. It works really well, and this personality type predominates in middle management. Occasionally people get in trouble for it (like when they are incompetent surgeons who's body count gets too high), but this happens rarely. Normally they are able to continue their destructive M.O. until they retire with huge bonuses and pensions.

     



  • The real WTF is spraying your code with deletes.

    C++ code should be written with RAII - no that isn't an American organisation that manages music licensing (that's RIAA). It's a computing technique to manage resources and their lifetimes, executed through the lifetime of other objects that depend on them.

    The only places a delete should ever be called is:

    1. A destructor
    2. An object or function object passed into a smart-pointer as a deleter. Thus if you are using shared_ptr with a custom deleter, your custom deleter can call delete.

    The correct logic would be to have a smart pointer member of the class (probably shared_ptr or unique_ptr), and "reset" this to a null.

     

     



  • @Cbuttius said:

    The real WTF is spraying your code with deletes.

     

    +1

     



  • @PJH said:

    Um, no. In C++ deleting a null pointer is a no-op (as is the case in C with free().) What is less forgivable is that this behaviour is the exception, rather than the rule. fclose(0) or close(-1) are undefined, for example.
    I know, that's why I wrote "decent": 'cause it should, like in C, where, at least in the olden days, free(NULL) would cause a bus error. Freeing NULL just isn't right. I haven't checked the ANSI spec on this issue, but my guess is it's probably still valid behavior.



  • @TGV said:

    @PJH said:

    Um, no. In C++ deleting a null pointer is a no-op (as is the case in C with free().) What is less forgivable is that this behaviour is the exception, rather than the rule. fclose(0) or close(-1) are undefined, for example.
    I know, that's why I wrote "decent": 'cause it should, like in C, where, at least in the olden days, free(NULL) would cause a bus error. Freeing NULL just isn't right. I haven't checked the ANSI spec on this issue, but my guess is it's probably still valid behavior.

    As stated by PJH:
    @PJH said:

    Um, no. In C++ deleting a null pointer is a no-op (as is the case in C with free().) What is less forgivable is that this behaviour is the exception, rather than the rule. fclose(0) or close(-1) are undefined, for example.

    I know this to be true as well. While I'm too lazy to look it up in the standard right now, here's a copy of a line from the free man page:
    @man free said:

    The free() function deallocates the memory allocation pointed to by ptr. If ptr is a NULL pointer, no
    operation is performed.


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.