Just ignore warnings



  • Developers in my department seem to think warnings are ignorable. I am constantly fixing warnings when I see them and have tried to explain that:

    1. A warning means you are probably doing something wrong.

    2. Even if it seems like a particular warning doesn't cause an issue, having thousands of warnings may bury one that is identifying an issue that is actually a bug.

    Here is the one I came across today:

    if ((strString.isEmpty()) || (strString == null))

    {

        ... do something

    }

    else

    {

       ... do something else

    }

    The warning said something along the lines of: check for null unnessessary, strString will never be null at this location.

    That warning is safe to ignore right? WTF?


  • ♿ (Parody)

    I have cow-orkers who leave tens of unused imports in their source files. I've largely given up, unless I need to edit something for other reasons. But some of these people can go for weeks without updating their svn working copy, so that's really pretty far down on the hierarchy of things I worry about any more.



  • @JoeCool said:

    if ((strString.isEmpty()) || (strString == null))

    Even without the warning this is a wtf...



  • @Weps said:

    @JoeCool said:

    if ((strString.isEmpty()) || (strString == null))

    Even without the warning this is a wtf...

    Yeah, no shit: an empty string is the same thing as a null string.



  • @morbiuswilters said:

    an empty string

    There's only one!



  • @Weps said:

    @JoeCool said:

    if ((strString.isEmpty()) || (strString == null))

    Even without the warning this is a wtf...

     

     

    #define strString strString == null) || (strString

     

    Fixed?



  • This is one thing that I like about C# (and any other language that has this) is the .IsEmptyOrNull()  and so handles "" and null so there is no confusion on what it says versus what it actually is doing.  The developer probably thought: IsEmpty() only checks "" and so I must manually check for null. 



  • @Anketam said:

    This is one thing that I like about C# (and any other language that has this) is the .IsEmptyOrNull()  and so handles "" and null so there is no confusion on what it says versus what it actually is doing.  The developer probably thought: IsEmpty() only checks "" and so I must manually check for null. 

    IsEmpty() does only check for "". The problem isn't that he was checking for null, the problem was that he was checking isEmpty() frist!

    The reason the compiler is saying that it will never be null there is because when isEmpty() is called, it will end up throwing a NullPointerException.


  • ♿ (Parody)

    @Anketam said:

    This is one thing that I like about C# (and any other language that has this) is the .IsEmptyOrNull()  and so handles "" and null so there is no confusion on what it says versus what it actually is doing.  The developer probably thought: IsEmpty() only checks "" and so I must manually check for null.&

    So, does C# allow method calls on a null object? In Java, you'd never get to the null check if you had a null, since you'd get a NullPointerException. Or am I being TRWTF for getting trolled or not understanding the OP?



  • @boomzilla said:

    So, does C# allow method calls on a null object? In Java, you'd never get to the null check if you had a null, since you'd get a NullPointerException. Or am I being TRWTF for getting trolled or not understanding the OP?

    It's a static method of the string type. So:

    string testing = null;
    if (string.IsNullOrEmpty(testing)) return 1;
    else return 0;
    

    will return 1.



  • @boomzilla said:

    @Anketam said:

    This is one thing that I like about C# (and any other language that has this) is the .IsEmptyOrNull()  and so handles "" and null so there is no confusion on what it says versus what it actually is doing.  The developer probably thought: IsEmpty() only checks "" and so I must manually check for null.&

    So, does C# allow method calls on a null object? In Java, you'd never get to the null check if you had a null, since you'd get a NullPointerException. Or am I being TRWTF for getting trolled or not understanding the OP?
    IsNullOrEmpty is static, so you call it as:

    if(String.IsNullOrEmpty(strString))

    ... but I've seen plenty of morons use it as:

    if(strString.IsNullOrEmpty(strString))

    It throws a compiler warning, but the people who do this sort of thing are also the same people who ignore warnings.



  • @Jaime said:

    It throws a compiler warning, but the people who do this sort of thing are also the same people who ignore warnings.

    I get a compiler error. "Member 'string.IsNullOrEmpty(string)' cannot be accessed with an instance reference; qualify it with a type name instead"


  • Considered Harmful

    I use Resharper and it would have have marked both "Possible NullReferenceException" and the always-false statement after it.

    Though, in C#, empty checks are static methods of the String class so it's harder to even make that kind of mistake [maybe if you tried strString.CompareTo( string.Empty ) there would be a similar issue].



  • @lettucemode said:

    @Jaime said:
    It throws a compiler warning, but the people who do this sort of thing are also the same people who ignore warnings.

     

    I get a compiler error. "Member 'string.IsNullOrEmpty(string)' cannot be accessed with an instance reference; qualify it with a type name instead"
    It's a warning in VB.Net, so I assumed C# would give a warning too.  Sorry.


  • Shame on me, I was thinking strictly static methods without realizing that he was using a member function to check, then again I never use .IsEmpty() since it takes more characters than writing: == "".



  • @Jaime said:

    It's a warning in VB.Net, so I assumed C# would give a warning too.  Sorry.

    I refuse your apology



  • @Jaime said:

    @lettucemode said:

    @Jaime said:
    It throws a compiler warning, but the people who do this sort of thing are also the same people who ignore warnings.

     

    I get a compiler error. "Member 'string.IsNullOrEmpty(string)' cannot be accessed with an instance reference; qualify it with a type name instead"
    It's a warning in VB.Net, so I assumed C# would give a warning too.  Sorry.
    Together now!

    VB is TRWTF



  • @boomzilla said:

    @Anketam said:

    This is one thing that I like about C# (and any other language that has this) is the .IsEmptyOrNull()  and so handles "" and null so there is no confusion on what it says versus what it actually is doing.  The developer probably thought: IsEmpty() only checks "" and so I must manually check for null.&

    you'd never get to the null check if you had a null, since you'd get a NullPointerException.
    This.

    In .net4, you now get new goodness of <FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas></FONT></FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>

    string</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas>.IsNullOrWhiteSpace()

    </FONT></FONT>which handles null, empty strings and white space...


  • @C-Octothorpe said:

    In .net4, you now get new goodness of <FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas></FONT></FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas> </FONT></FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>string</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas>.IsNullOrWhiteSpace()</FONT></FONT></FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas>

    </FONT></FONT>which handles null, empty strings and white space...

    I make my own String wrapper library:

    class Strings {
      public static boolean isUseless(String str) {
        return str == null || str.isEmpty() || str.isWhiteSpace() || str.isNothingINeed();
      }
    }
    
    (kidding of course)


  • @zelmak said:

    @C-Octothorpe said:
    In .net4, you now get new goodness of <FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas></FONT></FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas> </FONT></FONT></FONT><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas><FONT color=#0000ff size=2 face=Consolas>string</FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas>.IsNullOrWhiteSpace()</FONT></FONT></FONT></FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas> </FONT></FONT><FONT size=2 face=Consolas><FONT size=2 face=Consolas>

    </FONT></FONT>which handles null, empty strings and white space...

    I make my own String wrapper library:

    class Strings {
      public static boolean isUseless(String str) {
        return str == null || str.isEmpty() || str.isWhiteSpace() || str.isNothingINeed();
      }
    }
    

    (kidding of course)

    Actually, more to boomzillas comment about static members, in .net, you can create extension methods which appears as an instance method.  The funny thing is that, as long as you don't reference "this" in the extension method, you can call it from a null pointer (instance). So you can call "nullString.MyExtensionMethodBiotch()", and as long as in the definition of "MyExtensionMethodBiotch", you don't use the "this" parameter, it'll work fine.

    [Awaiting the barrage of "THATS BECAUSE ALL METHODS ARE STATIC MORAN" comments...]



  • @JoeCool said:

    The warning said something along the lines of: check for null unnessessary, strString will never be null at this location.
     

    Thanks for the warning.  I'll be sure and remember it when a maintenance programmer comes along here three years from now and changes one of the occurrences of strString to some other variable, but for now I'm leaving my "unnessessary" check where it is.



  • @da Doctah said:

    @JoeCool said:

    The warning said something along the lines of: check for null unnessessary, strString will never be null at this location.
     

    Thanks for the warning.  I'll be sure and remember it when a maintenance programmer comes along here three years from now and changes one of the occurrences of strString to some other variable, but for now I'm leaving my "unnessessary" check where it is.

    Are you serious? You shouldn't be coding if you think that there is nothing wrong with the code and you should just ignore the warning. Here is a hint (although if you read the thread and didn't understand what the problem is already, I feel sad for you): follow the code through as if strString is null... do you think the code does what was intended?



  • @morbiuswilters said:

    @Weps said:
    @JoeCool said:

    if ((strString.isEmpty()) || (strString == null))

    Even without the warning this is a wtf...

    Yeah, no shit: an empty string is the same thing as a null string.

    Ummm... no it isn't. Here is one example: Lets say you have a webservice that takes an optional parameter. When it comes through as null, that means it wasn't specified and you should do the default behaviour. When it comes through as an empty string, that means that the caller specified the value as empty on purpose. This might be useful, for example, if you want to clear a value vs. not changing it at all.



  • @morbiuswilters said:

    @JoeCool said:

    Yeah, no shit: an empty string is the same thing as a null string.

    Ummm... no it isn't. Here is one example: Lets say you have a webservice that takes an optional parameter. When it comes through as null, that means it wasn't specified and you should do the default behaviour. When it comes through as an empty string, that means that the caller specified the value as empty on purpose. This might be useful, for example, if you want to clear a value vs. not changing it at all.

    Do you know how I know you didn't read the tags?



  • I suspect his sarcasm detector is as busted as your quoting mechanism[1].

    In a previous life I assisted a group running a large bloated PHP webapp, which contained all the shite Blakey rants about when discussing open-source apps. It really was a stewpot of antipatterns, copy-pasta code[2] and WTFs that suggested a large group of people put it together whilst still maintaining strict silo mentality. It worked more by luck than effort, but Apache error_log grew to near a gig per day with warnings (50,000 registered site menbers, mind).  I warned them several times about these warnings, but since the site seemed to be working, fixing them weren't a priority (see [2]).

    Then they upgraded PHP, and those warnings became show-stopping errors.

    Suddenly, there was immense pressure to rush in and fix things. Yes, I know how to fix them. Sorry, I'm not available this week, nor next. I was available for the past six months, but I've recently moved house and my time will now be limited, so you'll have to get by without me. Catch you later...

    [1] yeah, I know - it's your deliberate mistake to feed dickweeds frothing at the opportunity to catch you out. Damn, it worked! How did that happen?

    [2] ironically, they were Italian coders, so most (object|function|method) names were untranslatable. Comments, however, could be translated - if they'd bothered to put any in. 



  • @Cassidy said:

    I suspect his sarcasm detector is as busted as your quoting mechanism[1].

    In a previous life I assisted a group running a large bloated PHP webapp, which contained all the shite Blakey rants about when discussing open-source apps. It really was a stewpot of antipatterns, copy-pasta code[2] and WTFs that suggested a large group of people put it together whilst still maintaining strict silo mentality. It worked more by luck than effort, but Apache error_log grew to near a gig per day with warnings (50,000 registered site menbers, mind).  I warned them several times about these warnings, but since the site seemed to be working, fixing them weren't a priority (see [2]).

    Then they upgraded PHP, and those warnings became show-stopping errors.

    Suddenly, there was immense pressure to rush in and fix things. Yes, I know how to fix them. Sorry, I'm not available this week, nor next. I was available for the past six months, but I've recently moved house and my time will now be limited, so you'll have to get by without me. Catch you later...

    [1] yeah, I know - it's your deliberate mistake to feed dickweeds frothing at the opportunity to catch you out. Damn, it worked! How did that happen?

    [2] ironically, they were Italian coders, so most (object|function|method) names were untranslatable. Comments, however, could be translated - if they'd bothered to put any in. 

    I once worked on a bloated PHP app with nearly 500k LoC (some was HTML, CS and JS, but still..) Of course, the only reason it was so big was because the "senior architect" insisted that "functions are slow" and copy-pasted the same bits of code all over the place. Actually, that's giving him too much credit. Instead, he manually typed the same code all over the place. How do I know? Because there were subtle typos and logic errors that weren't shared between the pieces of code. Also, a bug fix against one instance of this code never got applied to the others.

    At some point, he had finally given in and started using functions. However, he still believed "functions are slow and our app has to be kick-ass and fast" so he would create uber-functions spanning thousands of lines that did totally different things depending on what the first parameter was.

    Finally, he adopted OOP. Meaning he would write a class with only a constructor. The constructor would behave like an uber-function, doing lots of different things depending on the first parameter. To use the "class" you would just instantiate the object, then throw it away. Of course, most of the parameters the uber-functions and uber-constructors operated on were globals. There were thousands of globals, although many represented the same values, just with slightly different spelling or naming conventions used in the identifier.

    One day, out of curiosity, I decided to turn on errors up to E_NOTICE. A random page load generated around 30,000 messages. I also decided to log the DB queries. On an average page (which did around 300 queries) around 10% of the queries would actually return an error. Of course, there was no error checking. A while later, some developer accidentally left E_NOTICE on in a production release. Fifteen minutes after pushing the update, Apache stopped cold. Digging around, I discovered the PHP error log had exceeded 2.1 GB in size, which was more than the signed 32-bit int representing the file size could handle.

    The "senior architect" eventually went on to work at a Very Large Silicon Valley company. And, truth be told, he was only the 2nd worst developer I've had to work with.



  • You've given me toothache from rigid jaw-clench after reading that.

    On a plus note, I'm no longer concerned tonight about losing a bottle-opener. Tomorrow may be a different matter.



  • @da Doctah said:

    @JoeCool said:

    The warning said something along the lines of: check for null unnessessary, strString will never be null at this location.
     

    Thanks for the warning.  I'll be sure and remember it when a maintenance programmer comes along here three years from now and changes one of the occurrences of strString to some other variable, but for now I'm leaving my "unnessessary" check where it is.

    I disagree. A check for an impossible conditions is a BAD thing. It causes the developer to ponder what they are missing / not understanding. With full warnings on, the hypothetical future change (where the string could be null) would be quickly identified by the warning occuring that a string may be null. And with "warnings as errors", you will not even be able to compile until you put the check bag in.

     Remember every unnecessary line of code is a bug!



  • @TheCPUWizard said:

    @da Doctah said:

    @JoeCool said:

    The warning said something along the lines of: check for null unnessessary, strString will never be null at this location.
     

    Thanks for the warning.  I'll be sure and remember it when a maintenance programmer comes along here three years from now and changes one of the occurrences of strString to some other variable, but for now I'm leaving my "unnessessary" check where it is.

    I disagree. A check for an impossible conditions is a BAD thing. It causes the developer to ponder what they are missing / not understanding. With full warnings on, the hypothetical future change (where the string could be null) would be quickly identified by the warning occuring that a string may be null. And with "warnings as errors", you will not even be able to compile until you put the check bag in.

     Remember every unnecessary line of code is a bug!

    People, it's not about can it be null or not before reaching that line, it's about changing the line to first check for null and then do || isempty on it.

    Now I'm not a Java person, so perhaps the isempty checks for null as well (which makes it vague as hell) but then you're even sillier discussing about future proofing and shit. And it's not like I'm the first one to point this out to you either...



  • @morbiuswilters said:

    Digging around, I discovered the PHP error log had exceeded 2.1 GB in size, which was more than the signed 32-bit int representing the file size could handle.
     

    Random off-topic question: is there a good reason for file size to be a signed type?


  • Discourse touched me in a no-no place

    @Someone You Know said:

    Random off-topic question: is there a good reason for file size to be a signed type?
    Not really, but historically, the functions that returned file sizes used a signed number, and they used negative numbers to indicate errors. (Because 640K is enough 31 bits could represent any size these functions could legitimately return.)



  • Indeed. I think they meant to check for null first.



  • @PJH said:

    @Someone You Know said:
    Random off-topic question: is there a good reason for file size to be a signed type?
    Not really, but historically, the functions that returned file sizes used a signed number, and they used negative numbers to indicate errors. (Because 640K is enough 31 bits could represent any size these functions could legitimately return.)

     Heck, I remember when 16 bits was considered enough, and even 8 bits for some very early magnetic (but non-disk) media!



  • @TheCPUWizard said:

    @PJH said:

    @Someone You Know said:
    Random off-topic question: is there a good reason for file size to be a signed type?
    Not really, but historically, the functions that returned file sizes used a signed number, and they used negative numbers to indicate errors. (Because 640K is enough 31 bits could represent any size these functions could legitimately return.)

     Heck, I remember when 16 bits was considered enough, and even 8 bits for some very early magnetic (but non-disk) media!

    I'm guessing you're going to yell at us about getting off your lawn after you take your meds and remember where you are?



  • @Gazzonyx said:

    @TheCPUWizard said:

    @PJH said:

    @Someone You Know said:
    Random off-topic question: is there a good reason for file size to be a signed type?
    Not really, but historically, the functions that returned file sizes used a signed number, and they used negative numbers to indicate errors. (Because 640K is enough 31 bits could represent any size these functions could legitimately return.)

     Heck, I remember when 16 bits was considered enough, and even 8 bits for some very early magnetic (but non-disk) media!

    I'm guessing you're going to yell at us about getting off your lawn after you take your meds and remember where you are?

    Nah, there is always a party on my lawn....

    Seriously, most people should remember FAT-16 and 256 block devices were quite common on the early drums.  If people are nice, I might even let them visit my collection of classics from the 1960's.



  • @irreal said:

    Now I'm not a Java person, so perhaps the isempty checks for null as well (which makes it vague as hell)...

    That's impossible. isEmpty is an instance method. A null object has no instance methods. Trying to call instance methods on a null object would result in an NPE.



  • @morbiuswilters said:

    @irreal said:
    Now I'm not a Java person, so perhaps the isempty checks for null as well (which makes it vague as hell)...

    That's impossible. isEmpty is an instance method. A null object has no instance methods. Trying to call instance methods on a null object would result in an NPE.

     My favorite !!!!  (C# shown)

     class Foo
    {
        public bool IsNull { return false; }
    }



  • @TheCPUWizard said:

    @morbiuswilters said:

    @irreal said:
    Now I'm not a Java person, so perhaps the isempty checks for null as well (which makes it vague as hell)...

    That's impossible. isEmpty is an instance method. A null object has no instance methods. Trying to call instance methods on a null object would result in an NPE.

     My favorite !!!!  (C# shown)

     class Foo
    {
        public bool IsNull { return false; }
    }

    My favorite (C++):

    
    #include<iostream>
    
    using namespace std;
    
    class Foo
    {
        public:    
        bool isNull()
        {
            return this == 0;
        }
    };
    
    int main()
    {
        Foo* foo = new Foo();
    
        cout << "foo: " << foo->isNull() << endl;
    
        Foo* bar;
    
        cout << "bar: " << bar->isNull() << endl;
    }


    Outputs:

    foo: 0
    bar: 1
    


  • @TheCPUWizard said:

    Seriously, most people should remember FAT-16 and 256 block devices were quite common on the early drums.
    Not FAT12?



  • @morbiuswilters said:

    My favorite (C++):

    #include<iostream>
    
    using namespace std;
    
    class Foo
    {
        public:    
        bool isNull()
        {
            return this == 0;
        }
    };
    
    int main()
    {
        Foo* foo = new Foo();
    
        cout << "foo: " << foo->isNull() << endl;
    
        Foo* bar;
    
        cout << "bar: " << bar->isNull() << endl;
    }


    Outputs:

    foo: 0
    bar: 1
    

    Can you explain this? I haven't programmed C++ in the better part of a decade, but I can't understand that output. You never allocated a new Foo* (the pointer is null), so how can you dereference and access its member isNull()? Foo* bar IS null and you shouldn't be able to do that. I need coffee, my brain hurts.


  • Discourse touched me in a no-no place

    @Gazzonyx said:

    Foo* bar IS null
    No - it's uninitialised. And it works because isNull() isn't actually accessing anything within the object that was (or not) passed. If you change the definition of class Foo to, say:

    class Foo
    {
        public:    
        bool isNull()
        {
            return x == 0;
        }
        int x;
    };
    

    then you will segfault when bar->isNull() gets called.



  • Two thoughts:

    • Isn't an uninitialized pointer always null? Or am I completely forgetting something I used to know?
    • Doesn't "this" refer to the object itself? I thought that it was a self-referencing pointer behind the scenes?

  • Discourse touched me in a no-no place

    @Gazzonyx said:

    Isn't an uninitialized pointer always null? Or am I completely forgetting something I used to know?
    The only time a variable of a type without a ctor (int, char, double etc.) not explicitly initialised has a known value (i.e. zero) is if it's global or static. The value of automatic variables not initialised is, well, uninitialised/unknown. (Classes, of course, have constructors which may be used to initialise them.)

    @Gazzonyx said:

    Doesn't "this" refer to the object itself? I thought that it was a self-referencing pointer behind the scenes?
    It's equivalent to a pointer, but it's not actually a member of the class. You can think of member functions having an invisible first parameter which is called 'this', e.g. to use the code from above:

    class Foo
    {
        public:    
        bool isNull(Foo* this)
        {
            return this == 0;
        }
    };
    
    [...]
    cout << "foo: " << isNull(&foo) << endl;
    


  • @PJH said:

    @Gazzonyx said:
    Foo* bar IS null
    No - it's uninitialised. And it works because isNull() isn't actually accessing anything within the object that was (or not) passed. If you change the definition of class Foo to, say:

    class Foo
    {
        public:    
        bool isNull()
        {
            return x == 0;
        }
        int x;
    };
    

    then you will segfault when bar->isNull() gets called.

     

     But it does the comparison this==0, which if uninitialized, is not guaranteed to be true. It could be 0xDOGFOOD or 0xCFCFCFCF.

    And what if you do this:

     Foo* foo = new Foo();

    delete foo;

    foo->isNull()

     Whatever happens to be in memory... so you could actually get back that it is not null without a segfault until you actually try to do something else with it

     


  • Discourse touched me in a no-no place

    @JoeCool said:

    But it does the comparison this==0, which if uninitialized, is not guaranteed to be true. It could be 0xDOGFOOD or 0xCFCFCFCF.
    Indeed:

    [pjh@thinkpad-pjh tmp]$ cat x.cpp
    #include<iostream>
    
    using namespace std;
    
    class Foo
    {
        public:
        bool isNull()
        {
            return this == 0;
        }
    };
    
    int main()
    {
        Foo* foo = new Foo();
    
        cout << "foo: " << foo->isNull() << endl;
    
        Foo* bar = (Foo*)0xDOGFOOD; // pretend that the contents of the memory is garbage
    
        cout << "bar: " << bar->isNull() << endl;
    }
    
    [pjh@thinkpad-pjh tmp]$ ./x
    foo: 0
    bar: 0
    

    The fact that some compilers result in "bar: 1" is a consequence of the memory that stores the value contained in bar just happening to have 0 in it. It is, however, not guaranteed to contain 0 unless {im|ex}plicitly initialised to zero, and one should not rely on this behaviour.

    @JoeCool said:

    And what if you do this:

     Foo* foo = new Foo();

    delete foo;

    foo->isNull()

     Whatever happens to be in memory... so you could actually get back that it is not null without a segfault until you actually try to do something else with it

    With the original code, it won't segfault since it's not accessing anything in the object, with my modification, it might segfault, since it will be accessing memory the process (strictly speaking) no longer owns.



  • @PJH said:

    With the original code, it won't segfault since it's not accessing anything in the object, with my modification, it might segfault, since it will be accessing memory the process (strictly speaking) no longer owns.

    Don't forget to account for compler optimizations that *may* make this "work".  I have seen similar code resolve down to complete compile time behaviour and a runtime of just outputting "constant" information.



  • @TheCPUWizard said:

    @PJH said:

    With the original code, it won't segfault since it's not accessing anything in the object, with my modification, it might segfault, since it will be accessing memory the process (strictly speaking) no longer owns.

    Don't forget to account for compler optimizations that may make this "work".  I have seen similar code resolve down to complete compile time behaviour and a runtime of just outputting "constant" information.


    I'm guessing that it also depends on how the OS handles freed memory? I've heard that some OSes don't actually free memory from a process and put it back to the pool until there is a good chunk of it that has been freed.



  • First and foremost, it depends on undefined behavior.



  • @PJH said:

    Indeed:

    /*snip*/
    

    class Foo
    {
    public:
    bool isNull()
    {
    return this == 0;
    }
    };

    int main()
    {
    /snip/

    Foo* bar = (Foo*)0xDOGFOOD; // pretend that the contents of the memory is garbage
    
    cout &lt;&lt; "bar: " &lt;&lt; bar-&gt;isNull() &lt;&lt; endl;
    

    }

    [pjh@thinkpad-pjh tmp]$ ./x
    foo: 0
    bar: 0

    The fact that some compilers result in "bar: 1" is a consequence of the memory that stores the value contained in bar just happening to have 0 in it. It is, however, not guaranteed to contain 0 unless {im|ex}plicitly initialised to zero, and one should not rely on this behaviour. 

    The line

    return this == 0;

    is not comparing the value of the pointer to zero! It is comparing the value of the pointer to whatever value represents "null" on that machine.

    That said, I'd have to go check the C++ language specs to see if the line

    Foo* bar;

    is guaranteed to set the value of that pointer to null or if it is indeed uninitialized. The fun thing is, there may actually be a platform in which

    Foo* bar = (Foo*)0xDOGFOOD;
    return bar == 0;  

    returns 1 (at least in C; I can't say I recall for certain if that could happen in C++).


     


  • Discourse touched me in a no-no place

    @too_many_usernames said:

    is not comparing the value of the pointer to zero!

    Buggeration. I'm sure I've made that self-same point on another thread recently. I, of course, omitted the "on machines where NULL is all bits zero" disclaimer.

    @too_many_usernames said:

    That said, I'd have to go check the C++ language specs to see if the line

    Foo* bar;

    is guaranteed to set the value of that pointer to null or if it is indeed uninitialized.

    It's an auto variable. It is uninitialised. '96 Draft, 8.5 [dcl.init]
    2 Automatic, register, static, and external variables of namespace scope can be initialized by arbitrary expressions involving literals and previously declared variables and functions. [Example:

    int f(int);

    int a = 2;

    int b = f(a);

    int c(b);

    —end example]



    [...]



    9 If no initializer is specified for an object, and the object is of (possibly cv-qualified) non-POD class type (or array thereof), the object shall be default-initialized; if the object is of const-qualified type, the underlying class type shall have a user-declared default constructor. Otherwise, if no initializer is specified for an object, the object and its subobjects, if any, have an indeterminate initial value(86); if the object or any of its subobjects are of const-qualified type, the program is ill-formed.





    (86) This does not apply to aggregate objects with automatic storage duration initialized with an incomplete brace-enclosed initializer list; see 8.5.1.


Log in to reply