Closed Bug 798564 Opened 7 years ago Closed 7 years ago

remove prtypes.h #includes in modules/libmar/

Categories

(Core :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: froydnj, Assigned: froydnj)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Attached patch patch (obsolete) — Splinter Review
Had to fix up some PR_STATIC_ASSERTS, too.
Assignee: nobody → nfroyd
Status: NEW → ASSIGNED
Attachment #668609 - Flags: review?(ehsan)
Comment on attachment 668609 [details] [diff] [review]
patch

Review of attachment 668609 [details] [diff] [review]:
-----------------------------------------------------------------

::: modules/libmar/src/mar_private.h
@@ -13,5 @@
>  
> -/* Code in this module requires a guarantee that the size
> -   of uint32_t and uint64_t are 4 and 8 bytes respectively. */
> -PR_STATIC_ASSERT(sizeof(uint32_t) == 4);
> -PR_STATIC_ASSERT(sizeof(uint64_t) == 8);

lol!
Attachment #668609 - Flags: review?(ehsan) → review+
Backed out because of Windows build failures: https://hg.mozilla.org/integration/mozilla-inbound/rev/a27a41c7d7e3
Let's try this again.  The main sticking point was a use of PRUnichar in
mar.h.  But AFAICS, this is just a matter of "PRUnicher is a handy wide-
character type" and it doesn't line up with the use of mar_wopen elsewhere.
Therefore, I propose using wchar_t here: the prototype it's used in is
platform-specific anyway, so I'd argue it's more correct than PRUnichar.
Attachment #668609 - Attachment is obsolete: true
Attachment #705846 - Flags: review?(ehsan)
Attachment #705846 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/mozilla-central/rev/e60e28878899
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
(In reply to Nathan Froyd (:froydnj) from comment #5)
> Therefore, I propose using wchar_t here: the prototype it's used in is
> platform-specific anyway, so I'd argue it's more correct than PRUnichar.

Mmmmm wchar_t is 32-bits wide on linux and mac builds.
(In reply to Mike Hommey [:glandium] from comment #8)
> (In reply to Nathan Froyd (:froydnj) from comment #5)
> > Therefore, I propose using wchar_t here: the prototype it's used in is
> > platform-specific anyway, so I'd argue it's more correct than PRUnichar.
> 
> Mmmmm wchar_t is 32-bits wide on linux and mac builds.

Indeed.  But the particular usage in this patch is restricted to XP_WIN.
(In reply to comment #9)
> (In reply to Mike Hommey [:glandium] from comment #8)
> > (In reply to Nathan Froyd (:froydnj) from comment #5)
> > > Therefore, I propose using wchar_t here: the prototype it's used in is
> > > platform-specific anyway, so I'd argue it's more correct than PRUnichar.
> > 
> > Mmmmm wchar_t is 32-bits wide on linux and mac builds.
> 
> Indeed.  But the particular usage in this patch is restricted to XP_WIN.

Might be worth adding a comment explaining why using wchar_t there is OK.
You need to log in before you can comment on or make changes to this bug.