40088 - fix signed vs unsigned comparison in mozilla/modules/plugin/nglsrc/nsPluginHostImpl.cpp

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect, P3)

Description

[David Avery]

original code: 
                  NS_IMETHODIMP nsPluginHostImpl::GetCookie(const char* 
inCookieURL, void* inOutCookieBuffer, PRUint32& inOutCookieSize)
.
.
2585                      nsString cookieString;
..
2613                      if (NS_FAILED(rv) || 
2614                          (((PRInt32) inOutCookieSize) < 
cookieString.Length())) {
2615                        return NS_ERROR_FAILURE;

nsString.Length is defined as a PRUint32 in nsString.h
so remove PRInt32 cast on inOutCookieSize which is already declared as PRUint32

patch:
diff -u -r1.115 nsPluginHostImpl.cpp
--- nsPluginHostImpl.cpp        2000/05/16 00:22:30     1.115
+++ nsPluginHostImpl.cpp        2000/05/22 03:50:04
@@ -2611,7 +2611,7 @@
   rv = cookieService->GetCookieString(uriIn, cookieString);
   
   if (NS_FAILED(rv) || 
-      (((PRInt32) inOutCookieSize) < cookieString.Length())) {
+      (inOutCookieSize < cookieString.Length())) {
     return NS_ERROR_FAILURE;
   }
   bufPtr = cookieString.ToCString((char *) inOutCookieBuffer,

Comment 1

[edburns]

I have applied your patch.  Waiting for approval from Brendan.

Ed

Comment 2

[Xiaobin Lu]

Attachment: cvs -n update

Comment 3

[Xiaobin Lu]

Attachment: cvs diff -u

Comment 4

[Xiaobin Lu]

Attachment: run cvs diff

Comment 5

[Xiaobin Lu]

Attachment: The file "nsPluginHostImpl.cpp" which is modified

Comment 6

[edburns]

Xiaobin, here are some comments:

1. Please post the cvs -n update as an Additional comment, not as an attachment.

2. Note that your nsPluginHostImpl.cpp is out of date, as evidenced by the
"Merging differences between" comment.  Whenever you see this message, you'll
need to merge in the diffs and make your file current.  To do this:

1. mv your copy of the file in question to another name, perhaps
nsPluginHostImpl.40088.cpp

2. do a cvs update in that directory, to get the new file.

3. do a cvs edit on the file in question

4. Merge in your changes to the file, being VERY CAREFUL to to obliterate anyone
else's changes.

Comment 7

[Xiaobin Lu]

Attachment: 40088.tar.gz contains the modified files

Comment 8

[Xiaobin Lu]

cvs -n update (in directory E:\PROJECTS\MOZILLA\MODULES\PLUGIN\NGLSRC)
cvs server: Updating .
RCS file: /cvsroot/mozilla/modules/plugin/nglsrc/nsPluginHostImpl.cpp,v
retrieving revision 1.154
retrieving revision 1.162
Merging differences between 1.154 and 1.162 into nsPluginHostImpl.cpp
M nsPluginHostImpl.cpp
U nsIPluginInstanceOwner.h
U nsPluginInstancePeer.cpp
U nsPluginViewer.cpp

Comment 9

[Xiaobin Lu]

cvs update (in directory E:\PROJECTS\MOZILLA\MODULES\PLUGIN\NGLSRC)
cvs server: Updating .
M nsPluginHostImpl.cpp
cvs server: Updating mozilla
cvs server: Updating mozilla/modules
cvs server: Updating mozilla/modules/plugin
cvs server: Updating mozilla/modules/plugin/nglsrc

*****CVS exited normally with code 0*****

Comment 10

[Xiaobin Lu]

Attachment: cvs diff -u

Comment 11

[Xiaobin Lu]

Attachment: cvs diff

Comment 12

[Xiaobin Lu]

Attachment: tar.gz containning file modified

Comment 13

[edburns]

This looks ok, but take out the commented out line.  You still need to get 
review and approval from [e-mail], since this is a plugins bug.

Comment 14

[Xiaobin Lu]

M nsPluginHostImpl.cpp
cvs server: Updating mozilla
cvs server: Updating mozilla/modules
cvs server: Updating mozilla/modules/plugin
cvs server: Updating mozilla/modules/plugin/nglsrc

Comment 15

[Xiaobin Lu]

Attachment: cvs diff (The latest)

Comment 16

[Xiaobin Lu]

Attachment: cvs diff -u(The latest)

Comment 17

[Xiaobin Lu]

Attachment: tar.gz containing file modified(The latest)

Comment 18

[Xiaobin Lu]

Based on Ed's suggestion, I removed the comment and posted the corresponding 
attachments which marked as "( the lastest)". This is the first time I use 
Buggzilla and I try my best to not mess up this page even though I did. I hope 
it was not too late. Thanks for your guys help and tolerance.

Comment 19

[av (gone)]

Seems right thing to do, r=av.

Comment 20

[Xiaobin Lu]

This bug is fixed.

Comment 21

[edburns]

Xiaobin, you can't mark it fixed until it's checked into the tree!

Comment 22

[edburns]

You have r=av.  Can you get a=brendan so we can check this into the trunk and
close this bug?

Comment 23

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

sr=shaver

(I still can't believe there was a tarball attached for a one-line diff, but I'm
trying to keep it from really getting to me.)

Comment 24

[Xiaobin Lu]

Ed,I will get it approved by Brendan.

Comment 25

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

You have r and sr: you can check in now.

Comment 26

[Xiaobin Lu]

Ed has checked the fix in the truck so I would like to change this bug to fixed.

Comment 27

[shrirang khanzode]

verified that the fix was checked in version 1.186 on lxr.