227074 - nsTraceRefcnt.h should not include stdio.h

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

nsTraceRefcnt.h is now part of the SDK (and should be working; see bug 226555).
 It includes <stdio.h>.  If we want to remove that, it would be better to do it
now rather than once SDK users start depending on our headers bringing in
stdio.h.  It should also speed up compilation a bit.

(This was needed in the old nsTraceRefcnt.h before the glue/base split, since it
had FILE* parameters for some functions, but it's not needed now.)

Comment 1

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

It's not going to speed up compilation much since nsString2.h includes stdio.h
as well, so anyone including the unfrozen string classes by nsString.h /
nsString2.h will get stdio.h as well.  This also makes it much easier to fix,
since fewer includes need to be added...

Comment 2

[Brendan Eich [:brendan]]

One way to avoid over-including stdio.h is to depend on the FILE macro (which is
always a macro, AFAICT -- but probably the standard does not require it to be a
macro rather than a typedef), and bracket uses of FILE with #ifdef FILE, so that
callers wanting the FILE-dependent declarations to be visible must include
stdio.h themselves, first.

Cc'ing darin, who has a plan for strings.  We should get some bugs on file there.

/be

Comment 3

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

I don't think the string headers actually *need* to include stdio.h for any
reason.  They just do...

Comment 4

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Attachment: patch

This is what I needed to compile with the include removed.  I should probably
test on another platform, although I could just let tinderbox tell me...

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 136506 [details] [diff] [review]
patch

Any reason not to include <> headers before "" ones?  That is standard
operating procedure where I come from.	It would also match, e.g., the location
of the <stdlib.h> include in the first file in the patch.

/be

Comment 6

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Attachment: patch

Comment 7

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 137261 [details] [diff] [review]
patch

I want to get this in so external users of this header don't depend on it
including stdio.h.

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 137261 [details] [diff] [review]
patch

Sure, Asa agrees with safe code-level fixes and hygiene improvements like this
being shotgunned in.  BLAM!

/be

Comment 9

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Fix checked in to trunk, 2003-12-12 15:02 -0800.