Port some of the base system to FreeBSD. Extracted from...

[Replaced by http://codereview.chromium.org/183016 ]

Port some of the base system to FreeBSD. Extracted from

http://codereview.chromium.org/172032

[wtc, 2009-08-29 23:37:54]

Still LGTM.

http://codereview.chromium.org/180037/diff/1/6
File base/process_util.h (right):

http://codereview.chromium.org/180037/diff/1/6#newcode17
Line 17: // FIXME(benl): this surely can't be needed, since we construct a
The results of my experiments are:

1. If I remove the forward declaration of struct kinfo_proc,
compilation fails.

2. The forward declaration of struct kinfo_proc is enough
for the std::vector<kinfo_proc> member below.  I don't know
why.

3. struct kinfo_proc is defined in <sys/sysctl.h>.  If I
replace the forward declaration by #include <sys/sysctl.h>,
it also works.

So I think we can replace this FIXME comment (by the way,
we use TODO instead of FIXME) by a comment that says this
forward declaration is enough and we don't need to include
<sys/sysctl.h>.

http://codereview.chromium.org/180037/diff/1/6#newcode57
Line 57: 
Nit: delete one blank line here.

http://codereview.chromium.org/180037/diff/1/3
File base/sys_info_posix.cc (right):

http://codereview.chromium.org/180037/diff/1/3#newcode72
Line 72: // FIXME(benl): I have no idea how to get this
Nit: use TODO(benl).

[Ben Laurie (Chromium), 2009-08-30 04:43:45]

http://codereview.chromium.org/180037/diff/1/6
File base/process_util.h (right):

http://codereview.chromium.org/180037/diff/1/6#newcode17
Line 17: // FIXME(benl): this surely can't be needed, since we construct a
On 2009/08/29 23:37:54, wtc wrote:
> The results of my experiments are:
> 
> 1. If I remove the forward declaration of struct kinfo_proc,
> compilation fails.
> 
> 2. The forward declaration of struct kinfo_proc is enough
> for the std::vector<kinfo_proc> member below.  I don't know
> why.
> 
> 3. struct kinfo_proc is defined in <sys/sysctl.h>.  If I
> replace the forward declaration by #include <sys/sysctl.h>,
> it also works.
> 
> So I think we can replace this FIXME comment (by the way,
> we use TODO instead of FIXME) by a comment that says this
> forward declaration is enough and we don't need to include
> <sys/sysctl.h>.

It's because the vector headers only refer to type*, not type itself.

http://codereview.chromium.org/180037/diff/1/6#newcode57
Line 57: 
On 2009/08/29 23:37:54, wtc wrote:
> Nit: delete one blank line here.

Done.

http://codereview.chromium.org/180037/diff/1/3
File base/sys_info_posix.cc (right):

http://codereview.chromium.org/180037/diff/1/3#newcode72
Line 72: // FIXME(benl): I have no idea how to get this
On 2009/08/29 23:37:54, wtc wrote:
> Nit: use TODO(benl).

Done.

[pvalchev, 2009-09-03 23:09:07]

Nice!

So I had started a very similar patch for OpenBSD:
http://codereview.chromium.org/118469

But I got really busy shortly after and went on vacation, I'd like to resume &
merge my patch too. The reason I didn't finish it is because I wanted to clean
it up and make a OS_BSD define like discussed in that patch with tony@, matching
all of the BSD's. Any chance we can do this? I see you are adding OS_FREEBSD ||
OS_LINUX one-offs everywhere and the same will have to be repeated for all of
the BSD's... it would be nice to make this cleaner.

(Several of my smaller patches that fixed linux-isms were already submitted and
benefit everyone)

[brettw, 2009-09-04 00:02:29]

What about
#if defined(OS_POSIX) && !defines(OS_LINUX)
?


On Thu, Sep 3, 2009 at 4:08 PM, <pvalchev@gmail.com> wrote:
> Nice!
>
> So I had started a very similar patch for OpenBSD:
> http://codereview.chromium.org/118469
>
> But I got really busy shortly after and went on vacation, I'd like to
> resume & merge my patch too. The reason I didn't finish it is because I
> wanted to clean it up and make a OS_BSD define like discussed in that
> patch with tony@, matching all of the BSD's. Any chance we can do this?
> I see you are adding OS_FREEBSD || OS_LINUX one-offs everywhere and the
> same will have to be repeated for all of the BSD's... it would be nice
> to make this cleaner.
>
> (Several of my smaller patches that fixed linux-isms were already
> submitted and benefit everyone)
>
>
> http://codereview.chromium.org/180037
>

[Ben Laurie (Chromium), 2009-09-04 10:04:14]

On Fri, Sep 4, 2009 at 1:02 AM, Brett Wilson<brettw@chromium.org> wrote:
> What about
> #if defined(OS_POSIX) && !defines(OS_LINUX)
> ?

That would include MacOS, but most of the ifs exclude it, so you'd end up with

#if defined(OS_POSIX) && !defined(OS_LINUX) && !defined(OS_MACOSX)

which doesn't seem like much of a win - and would get longer if we
added any other POSIX-like system that wasn't BSD-like.

>
>
> On Thu, Sep 3, 2009 at 4:08 PM, <pvalchev@gmail.com> wrote:
>> Nice!
>>
>> So I had started a very similar patch for OpenBSD:
>> http://codereview.chromium.org/118469
>>
>> But I got really busy shortly after and went on vacation, I'd like to
>> resume & merge my patch too. The reason I didn't finish it is because I
>> wanted to clean it up and make a OS_BSD define like discussed in that
>> patch with tony@, matching all of the BSD's. Any chance we can do this?
>> I see you are adding OS_FREEBSD || OS_LINUX one-offs everywhere and the
>> same will have to be repeated for all of the BSD's... it would be nice
>> to make this cleaner.
>>
>> (Several of my smaller patches that fixed linux-isms were already
>> submitted and benefit everyone)
>>
>>
>> http://codereview.chromium.org/180037
>>
>

[Ben Laurie (Chromium), 2009-09-04 10:07:08]

On Fri, Sep 4, 2009 at 12:09 AM, <pvalchev@gmail.com> wrote:
> Nice!
>
> So I had started a very similar patch for OpenBSD:
> http://codereview.chromium.org/118469
>
> But I got really busy shortly after and went on vacation, I'd like to
> resume & merge my patch too. The reason I didn't finish it is because I
> wanted to clean it up and make a OS_BSD define like discussed in that
> patch with tony@, matching all of the BSD's. Any chance we can do this?
> I see you are adding OS_FREEBSD || OS_LINUX one-offs everywhere and the
> same will have to be repeated for all of the BSD's... it would be nice
> to make this cleaner.

Sure, I'd be happy to go down this route. Presumably you'd still want
there to be an OS_FREEBSD and OS_OPENBSD as well, for the, presumably
few, places where they differ?

> (Several of my smaller patches that fixed linux-isms were already
> submitted and benefit everyone)
>
>
> http://codereview.chromium.org/180037
>