me vs. VS2022.. detecting NULL pointers

charlieg

So, I'm lifting a VC6++ code base to VS2022. It's tedious. Lot's of landmine opportunities, and to say VS2022 is more particular is an understatement... but this code snippet has me mystified - here is the beginning of my original code and it throws a "Warning C6011" possible NULL pointer. Okay, so I see that.

void CInfoBar::SetText(LPCTSTR lpszNew)
{
ASSERT(lpszNew && AfxIsValidString(lpszNew));

if (\*lpszNew == \_T('#'))
{
    TCHAR szNum\[8\];
    INT nCount = 0;
    .
    .
    .

per the help page, it encourages me to check the pointer passed before using it. So I changed the code to below (wrapped it in an if statement.

if (NULL != lpszNew)
{
ASSERT(lpszNew && AfxIsValidString(lpszNew));

   if (\*lpszNew == \_T('#'))
   {
       TCHAR szNum\[8\];
       INT nCount = 0;
       .
       .
       .

It still throws the warning. I only have a couple of hundred of these things in code the original developer lifted from Microsoft 20 years ago, so... :)

Charlie Gilley “They who can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety.” BF, 1759 Has never been more appropriate.

charlieg

oh ffs. In a small spot of inspiration, I changed my if statement from: "

if (NULL != lpszNew)
{

to

if (lpszNew != NULL)
{

and the warning goes away. The first form is a time honored coding tradition to prevent typos in conditionals. You obviously cannot assign a value to NULL, so you catch it when you mistype the conditional. Fine, I'm moving on. In about 6 months, I'm going to be fishing.

Charlie Gilley “They who can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety.” BF, 1759 Has never been more appropriate.

Mircea Neacsu

#pragma wanring (disable: 6011)

If you know your code is working, you can dispense with the VS silliness.

Mircea

Richard Andrew x64

It's nice to know that even well-seasoned developers like you can have these kinds of moments. I thought I was alone. :( :)

The difficult we do right away... ...the impossible takes slightly longer.

charlieg

As the above poster said, I'm a well seasoned developer inheriting a code base that no one has looked at in 20 years. Since I'm not retired or dead yet... point taken on warnings, but I have been burned too many times by other developers turning off warnings. I take the phrase "Here be dragons..." seriously. Still I'm amazed that the MS compiler would distinguish between either version of the source.

Charlie Gilley “They who can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety.” BF, 1759 Has never been more appropriate.

Mircea Neacsu

charlieg wrote:

I take the phrase "Here be dragons..." seriously.

And so you should. Seems the author of the code in question also took it seriously:

void CInfoBar::SetText(LPCTSTR lpszNew)
{
ASSERT(lpszNew && AfxIsValidString(lpszNew));

In this case assert seems the proper way to handle a failing precondition: the function cannot work with a NULL pointer. Fullstop! In the new variant something has changed:

if (NULL != lpszNew)
{
ASSERT(lpszNew && AfxIsValidString(lpszNew));

The ASSERT after the if is clearly not needed (or it could be simplified to keep just the second condition). Also, more importantly the contract conditions of the function have changed: previously the function said "I cannot deal with NULL values for lpszNew". Now it says "if you pass me a NULL value I'm going to do something about it (possibly an else clause for the if statement or something)". I'm not saying this is wrong but is something to consider carefully. PS:

charlieg wrote:

Still I'm amazed that the MS compiler would distinguish between either version of the source.

It's the artificial intelligence :laugh:

Mircea

trønderen

charlieg wrote:

The first form is a time honored coding tradition to prevent typos in conditionals.

I wonder if anyone ever has remembered to write the conditional backwards, yet used '=' rather than '=='. My impression is that those who turns the condition around never mixes up assignment and equality operators. I prefer the natural way of writing conditionals, emphasizing readability over dogmas. (I never say 'if 1979 is the vintage', but 'if the vintage is 1979', or 'if green is the car' but 'if the car is green'.) And, I can't remember as far back as to when the C compiler did not give a warning at 'if (lpszNew != NULL)', something like 'Possibly unintended assignment'.

Religious freedom is the freedom to say that two plus two make five.

Richard Andrew x64

trønderen wrote:

those who turns the condition around never mixes up assignment and equality operators.

Well, then the backwards writing of the conditional served its purpose, right? If it makes you give extra attention to the code, then that's a good thing.

The difficult we do right away... ...the impossible takes slightly longer.

jschell

trønderen wrote:

I wonder if anyone ever has remembered to write the conditional backwards, yet used '=' rather than '=='.

Probably not enough to agonize over it.

trønderen wrote:

I prefer the natural way of writing conditionals

I do too. Every time I see the other form my brain tends to do a little twist as I try to figure it out. And of course these days if one is really concerned then add a method 'IsNull()' and use that. That makes it really obvious.

trønderen

At the cost of reduced readability. No, I prefer more readable code. And I trust that the compiler will warn me about 'Possibly unintended assignment' - as it does, maybe once every 2-3 years.

Religious freedom is the freedom to say that two plus two make five.

Mircea Neacsu

trønderen wrote:

I never say 'if 1979 is the vintage'

... because a Jedi master you are not :D

Mircea

CPallini

trønderen wrote:

I wonder if anyone ever has remembered to write the conditional backwards, yet used '=' rather than '=='. My impression is that those who turns the condition around never mixes up assignment and equality operators.

Right. However I sometimes make a typo.

Quote:

I prefer the natural way of writing conditionals,emphasizing readability over dogmas

Me too. Luckily, modern compilers produce a warning.

"In testa che avete, Signor di Ceprano?" -- Rigoletto