Remove use of PRInt32 in Keychain code

RESOLVED FIXED

Status

--
trivial
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: batwood.bugs, Assigned: batwood.bugs)

Tracking

({fixed1.8.1.12})

Details

Attachments

(1 attachment, 1 obsolete attachment)

12.52 KB, patch
stuart.morgan+bugzilla
: review+
Details | Diff | Splinter Review
(Assignee)

Description

11 years ago
Functions in KeychainService.* (and upcoming KeychainAutoCompleteSession.*)
 use PRInt32 type but this should be changed to use something more Cocoa-y
(Assignee)

Comment 1

11 years ago
Created attachment 298676 [details] [diff] [review]
Patch to change PRInt32 to UInt32

I had prepared this on Mento's suggestion to change PRInt32 method arguments to UInt32.  Instead of carrying around -1 for the port value, it's quickly set to kAnyPort since UInt32 is unsigned.
Attachment #298676 - Flags: review?(stuart.morgan)

Updated

11 years ago
Attachment #298676 - Flags: superreview?(mark)
Attachment #298676 - Flags: review?(stuart.morgan)
Attachment #298676 - Flags: review+

Updated

11 years ago
Attachment #298676 - Flags: superreview?(mark) → superreview+
(Assignee)

Updated

11 years ago
Flags: camino1.6b3?
Keywords: checkin-needed
Checked in on the trunk and MOZILLA_1_8_BRANCH.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Flags: camino1.6b3?
Keywords: checkin-needed → fixed1.8.1.12
Resolution: --- → FIXED

Comment 3

11 years ago
Created attachment 299837 [details] [diff] [review]
Missed a kAnyPort

Brian, I didn't see this one because it was cut out by patch context - actually, in the patch context, it looked kind of right.  ExtractRealmComponents had its port (out-)parameter changed from a PRInt32 to a UInt16, but it's still setting it to -1.  Is kAnyPort correct to use here?

ExtractRealmComponents was a protected static member function in KeychainPrompt, but it didn't even need that kind of visibility.  In this patch, I've made it a simple static function, and have given its buddies PreFill and ProcessPrompt the same treatment.  (The latter two weren't even static, although they didn't operate on an object.)
Attachment #299837 - Flags: superreview?(stuart.morgan)
Attachment #299837 - Flags: review?(batwood.bugs)

Updated

11 years ago
Attachment #299837 - Flags: superreview?(stuart.morgan) → superreview+
Comment on attachment 299837 [details] [diff] [review]
Missed a kAnyPort

Obsoleting this patch; Stuart landed the kAnyPort at some point, and the static stuff now has its own bug, bug 425258.
Attachment #299837 - Attachment is obsolete: true
Attachment #299837 - Flags: review?(batwood.bugs)
You need to log in before you can comment on or make changes to this bug.