798564 - remove prtypes.h #includes in modules/libmar/

Status: RESOLVED FIXED

(Core :: General, defect)

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: patch

Had to fix up some PR_STATIC_ASSERTS, too.

Comment 2

[(no longer active)]

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!

Comment 3

[Nathan Froyd [:froydnj]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/310e29c26ea3

Comment 4

[(no longer active)]

Backed out because of Windows build failures: https://hg.mozilla.org/integration/mozilla-inbound/rev/a27a41c7d7e3

Comment 5

[Nathan Froyd [:froydnj]]

Attachment: remove prtypes.h #includes in modules/libmar/

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.

Comment 6

[Nathan Froyd [:froydnj]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e60e28878899

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e60e28878899

Comment 8

[Mike Hommey [:glandium]]

(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.

Comment 9

[Nathan Froyd [:froydnj]]

(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.

Comment 10

[(no longer active)]

(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.