mismatching delete

If it doesn't fit anywhere else, drop it in here. (not to be used as a chat/nonsense section)

Moderator: Moderator Team

Post Reply
maddin200
Posts: 3
Joined: Thu Jul 23, 2015 10:50 am

mismatching delete

Post by maddin200 »

\base\shell\explorer\syspager.cpp line 479
should be:
if (WatchList)
delete [] WatchList;
User avatar
Fraizeraust
Posts: 234
Joined: Thu Jan 05, 2017 11:46 am
Location: Italy
Contact:

Re: mismatching delete

Post by Fraizeraust »

I'm not a C++ geek programmer myself but is there a reason for why delete needs to be replaced with the delete array operator delete[]? If WatchList variable is declared from the code with new[] as a set of an array of objects then undoubtedly delete[] should be called (as according to Stack Overflow website). Otherwise just keep the operator as it's.
a.k.a. GeoB99 -- ReactOS Kernel developer -- My Wiki page
ROCKNROLLKID
Posts: 307
Joined: Mon Oct 17, 2016 3:19 am
Contact:

Re: mismatching delete

Post by ROCKNROLLKID »

You should probably submit your patches to here: https://github.com/reactos/reactos
justincase
Posts: 441
Joined: Sat Nov 15, 2008 4:13 pm

Re: mismatching delete

Post by justincase »

I'm not exactly a C++ guru either, but according to the standard (yes, I looked it up) delete something is "for non-array objects", and delete [] something is "for arrays". It then goes on to specify that when used as delete something, "the value of the operand of delete may be a null pointer
value, a pointer to a non-array object created by a previous new-expression, or a pointer to a subobject representing a base class of such an object. If not, the behavior is undefined." and that when used as delete [] something "the value of the operand of delete may be a null pointer value or a pointer
value that resulted from a previous array new-expression. If not, the behavior is undefined."
Also, just to be clear, they include the following note: "[ Note: this means that the syntax of the delete-expression must match the type of the object allocated by new, not the syntax of the new-expression. — end note ]".

Now, looking at excerpts from the code in question, separated by "...", annotated in comments (after "\\") at the end of the associated line.

Code: Select all

UINT WINAPI CIconWatcher::WatcherThread(_In_opt_ LPVOID lpParam)
{
    ...
    HANDLE *WatchList = NULL; \\ Here, WatchList is created as a "null pointer value", deletable with either syntax

    This->m_Loop = true;
    while (This->m_Loop)
    {
        ...
        if (WatchList)
            delete WatchList; \\ Here, WatchList is deleted with 'delete object' syntax
        WatchList = new HANDLE[Size]; \\ Here, WatchList is created as an array, which should only be deleted by 'delete array' syntax
        ...
    }

    if (WatchList)
        delete WatchList; \\ Here, WatchList is deleted with 'delete object' syntax

    ...
}
Therefore I think, based on my understanding of what this code does, that whenever this code is run, we're hitting the dreaded "undefined behavior". :o
(Edit... removed "while we have more than one icon in the system tray", it will always go through the loop at least once, which will make it into an array, and both deletes use the wrong syntax for that.)

@maddin200: Please submit a pull-request on GitHub, as ROCKNROLLKID suggested.
Last edited by justincase on Sun Feb 11, 2018 9:25 pm, edited 1 time in total.
I reserve the right to ignore any portion of any post if I deem it not constructive or likely to cause the discussion to degenerate.
maddin200
Posts: 3
Joined: Thu Jul 23, 2015 10:50 am

Re: mismatching delete

Post by maddin200 »

Hi, would be faster if somebody with more knowledge of the source code file provide the patch.

Bye Martin
User avatar
Fraizeraust
Posts: 234
Joined: Thu Jan 05, 2017 11:46 am
Location: Italy
Contact:

Re: mismatching delete

Post by Fraizeraust »

I do not see the reason why you can't push the patch yourself, since the ReactOS codebase is in GitHub, using GitHub is quite easy and less hassle. But as you wish, maddin200, I will create the PR just for you but bear in mind I will also mention you in the PR description as the main guy who took note of this.

Pull Request: https://github.com/reactos/reactos/pull/374
a.k.a. GeoB99 -- ReactOS Kernel developer -- My Wiki page
hbelusca
Developer
Posts: 1204
Joined: Sat Dec 26, 2009 10:36 pm
Location: Zagreb, Croatia

Re: mismatching delete

Post by hbelusca »

PR committed in abdde0b. Thanks for the report! But for the future, please create GitHub PRs and/or report on JIRA. Almost no developer look at the forums nowadays.
Post Reply

Who is online

Users browsing this forum: No registered users and 19 guests