Bug 311611 - Null comparison always yields false warning on an assignment
Summary: Null comparison always yields false warning on an assignment
Status: CLOSED WORKSFORME
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.5.2   Edit
Hardware: PC Windows XP
: P3 minor (vote)
Target Milestone: 3.6 RC3   Edit
Assignee: Ayushman Jain CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-04 15:35 EDT by Ryan Gross CLA
Modified: 2010-07-30 12:07 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ryan Gross CLA 2010-05-04 15:35:51 EDT
When compiling the following source code: 

NativeBuffer headerData = null;
			int prevByteSize = 0;                   // size of byte array created in the previous pass
			try {
				while(pixel_offset == -1) {
					// Murali Kumaran. This is added to prevent the OutOfMemory error
					// or crash of the workstation, if the application could not find
					// the end of the header file.
					// If the maxHeaderSizeExpected is set to a value <=0 then
					// the application will continue until it find an end of
					// file or an error.

					if (maxHeaderSizeExpected > 0) {
						if (byteSize >= maxHeaderSizeExpected) {
							// Cleanup the native buffer used to calculate the header size.
							if( headerData != null ) {
								headerData.dispose();
								headerData = null;
							}
							throw new IOException("[ISUImagePeer] Could not find the end of header file after parsing [" + byteSize + "] Bytes");
						}
					}

					//		          IISge49600 Add check for image file size
					if (byteSize >= imageFileSize ) {
						// Cleanup the native buffer used to calculate the header size.
						if( headerData != null ) {
							headerData.dispose();
							headerData = null;
						}
						throw new IOException("[ISUImagePeer] Could not find the end of header after parsing [" + byteSize + "] Bytes, with imageFileSize [" + imageFileSize + "]");
					}

					byteSize *= 2;                                  // double byteSize
					// 				 IISge49600 Add check for image file size
					if(byteSize > imageFileSize)
					{
						byteSize = imageFileSize;
					}

					NativeBuffer prevHeaderData = headerData;
					headerData = null;
					if( prevHeaderData != null ) {
						prevByteSize = prevHeaderData.getSize();
					}
					try {
						headerData = allocateTempMemory(byteSize);
						if(prevHeaderData != null)  {                     // copy the previously read data into the new block
							prevHeaderData.copyToNativeBuffer(0,prevByteSize,headerData,0);
						}
					}
					finally {
						prevHeaderData.dispose();
						prevHeaderData = null;
					}

a compiler warning with the text "Null comparison always yields false: The variable prevHeaderData cannot be null at this location" is generated for the last line, which is actually an assignment.

-- Configuration Details --
Product: Eclipse 1.2.2.20100122-1337 (org.eclipse.epp.package.java.product)
Installed Features:
 org.eclipse.jdt 3.5.2.r352_v20100108-7r88FEwFI0WTuoBl0iaG0tyhfZH6
Comment 1 Walter Harley CLA 2010-05-05 00:50:24 EDT
Can you provide a code sample that reproduces the problem and that can actually be compiled?  

It is not possible to compile or comprehend this incomplete example, so there is no way for us to test or reproduce the problem.
Comment 2 Walter Harley CLA 2010-05-05 01:00:07 EDT
By the way, it may or may not be relevant, but I should point out a bug in this code (perhaps this is what the compiler is trying to warn about): if prevHeaderData is null, the first line of the finally{} block will throw a NullPointerException.

And, there is no point in setting prevHeaderData to null; it is presumably about to be out of scope anyway (and if it's not, then enclosing that chunk of code in a {} block will have the same effect).  All that setting it to null will do is waste a few processor cycles (though hopefully the JIT will optimize it out anyway).  If you run Findbugs on this you'll probably get a Dead Store warning.

The above is guesswork since I can't see the complete method (nor even the complete loop).
Comment 3 Ayushman Jain CLA 2010-05-06 14:14:03 EDT
Ryan, as Walter pointed out, a working code will help us in figuring out the exact problem, if any.

I tried to reproduce the error by using your code and using quick fixes to clean up all the errors. Once I was done, i obtained a null check warning on the first line of the finally block ie. on 'prevHeaderData.dispose();' - Potential null pointer access: The variable prevHeaderData may be null at this location. This warning is expected and correct.(Walter correctly mentions this in comment 2). I, however, could not obtain the warning that you mentioned here.

In order to proceed further, I request two things:
1) If possible, can you provide a working error-free code (possibly a minimal piece) that could reproduce this problem?
2) Can you download a newer build and check if you still have the same problem?

Thanks!
Comment 4 Ayushman Jain CLA 2010-05-24 06:15:32 EDT
(In reply to comment #0)
Sorry, I just noticed that the version is 3.5.2. I'd tried on 3.6. 
So yes I could reproduce this on 3.5.2, but since its fixed in 3.6, can you upgrade to 3.6?
Ryan, let me know if its a problem. Thanks!
Comment 5 Ayushman Jain CLA 2010-05-24 06:54:37 EDT
Closing as WORKSFORME. 3.5.2 development is closed and so this cannot be fixed on 3.5.2
Ryan, you can do either of the two:
1) Download a new 3.6 build.
2) Disable the redundant null check warnings in the Preferences->java->compiler->"errors/warnings"->unnecessary code section.
Comment 6 Frederic Fusier CLA 2010-06-01 05:46:37 EDT
Verified for 3.6RC3 using build I20100527-1700
Comment 7 Ryan Gross CLA 2010-07-30 12:07:53 EDT
The code now shows the correct aerror message now