Closed Bug 1013675 Opened 6 years ago Closed 6 years ago

Implement nsDebugImpl::GetIsDebuggerAttached() on BSDs

Categories

(Core :: XPCOM, defect)

All
FreeBSD
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: jbeich, Assigned: jbeich)

Details

Attachments

(1 file)

GetIsDebuggerAttached() seems to be used by bug 763138 and bug 910863. I'm not sure why Linux doesn't implement it but the code for BSDs is quite similar to OS X.
Attachment #8425924 - Flags: review?(benjamin)
Landry, can you push the patch here and in bug 1012415 to Try (build-only) ?
Flags: needinfo?(landry)
The patch is nop for Linux. I need Try build on at least OS X.
Flags: needinfo?(landry)
Would have been better to say it before... and really, you should get try access.

https://tbpl.mozilla.org/?tree=Try&rev=4b945e23e0ad
Flags: needinfo?(landry)
Comment on attachment 8425924 [details] [diff] [review]
copypasta from ipc/chromium/src/base/debug_util_dposix.cc

Going to defer this to vladan since he wrote the code originally.
Attachment #8425924 - Flags: review?(benjamin) → review?(vdjeric)
Comment on attachment 8425924 [details] [diff] [review]
copypasta from ipc/chromium/src/base/debug_util_dposix.cc

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

Did you try building this patch on these BSD platforms?

::: xpcom/base/nsDebugImpl.cpp
@@ +50,3 @@
>  #include <stdbool.h>
>  #include <unistd.h>
> +#include <sys/param.h>

Do we need sys/param.h?
Attachment #8425924 - Flags: review?(vdjeric) → review+
(In reply to Vladan Djeric (:vladan) from comment #6)
> Did you try building this patch on these BSD platforms?

Yes, I've tested build and runtime on BSDs using a simple .c file based on the diff.

> ::: xpcom/base/nsDebugImpl.cpp
> @@ +50,3 @@
> >  #include <stdbool.h>
> >  #include <unistd.h>
> > +#include <sys/param.h>
> 
> Do we need sys/param.h?

Because on OpenBSD sysctl(3) manpage recommends so and the following error:

In file included from test.c:9:
/usr/include/sys/proc.h:65: error: 'MAXLOGNAME' undeclared here (not in a function)
/usr/include/sys/proc.h:92: error: expected ')' before 'int'
/usr/include/sys/proc.h:93: error: expected ';' before 'int'
/usr/include/sys/proc.h:322: error: 'MAXCOMLEN' undeclared here (not in a function)
/usr/include/sys/proc.h:331: error: field 'p_sigstk' has incomplete type
/usr/include/sys/proc.h:344: error: field 'p_sigval' has incomplete type

NetBSD also recommends including sys/param.h with sys/sysctl.h while OS X, DragonFly and FreeBSD recommend sys/types.h. sys/param.h (unlike sys/types.h) is rarely implicitly included by non sys/* headers and style(9) on some BSDs says:

     Kernel include files (i.e. sys/*.h) come first; normally, include
     <sys/types.h> OR <sys/param.h>, but not both.  <sys/types.h> includes
     <sys/cdefs.h>, and it is okay to depend on that.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e450502ae19e
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.