Implement PR_GetPhysicalMemorySize on FreeBSD

RESOLVED FIXED in 4.10.8

Status

NSPR
NSPR
P1
normal
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: Jan Beich, Assigned: Jan Beich)

Tracking

(Blocks: 1 bug)

4.9.2
4.10.8
All
FreeBSD
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 2 obsolete attachments)

Comment hidden (empty)
(Assignee)

Comment 1

6 years ago
Created attachment 651202 [details] [diff] [review]
reuse code for (Net|Open)BSD
Attachment #651202 - Flags: review?(wtc)
(Assignee)

Updated

6 years ago
Blocks: 512076
(Assignee)

Comment 2

6 years ago
Created attachment 651203 [details] [diff] [review]
reuse code for other BSDs, v2

Now it should work on 32bit systems. Sorry.
Attachment #651202 - Attachment is obsolete: true
Attachment #651202 - Flags: review?(wtc)
Attachment #651203 - Flags: review?(wtc)
(Assignee)

Comment 3

6 years ago
Created attachment 652733 [details] [diff] [review]
reuse code for other BSDs, v3

Keep extra ifdef simple in case more BSD forks are added or FreeBSD/DragonFly grow support for hw.physmem64:

  #ifdef HW_PHYSMEM64
Attachment #651203 - Attachment is obsolete: true
Attachment #651203 - Flags: review?(wtc)
Attachment #652733 - Flags: review?(wtc)

Updated

4 years ago
Blocks: 1111041

Updated

4 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Hardware: x86_64 → All
Lacking this support is starting to seriously hurt us on FreeBSD now that we are using PR_GetPhysicalMemorySize more. Can we move forward on getting a review for this?
Flags: needinfo?(wtc)

Comment 6

4 years ago
Comment on attachment 652733 [details] [diff] [review]
reuse code for other BSDs, v3

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

r=wtc. Thanks for the patch.

Patch checked in on the NSPR trunk:
https://hg.mozilla.org/projects/nspr/rev/1ec32b5680f7

::: nsprpub/pr/src/misc/prsystem.c
@@ +300,3 @@
>      uint64_t memSize;
> +#else
> +    unsigned long memSize;

I found some sample code on the Web that declares this variable as
unsigned int. Which type is correct?
Attachment #652733 - Flags: review?(wtc)
Attachment #652733 - Flags: review+
Attachment #652733 - Flags: checked-in+

Comment 7

4 years ago
This fix will be in the upcoming NSPR 4.10.8 release.
Assignee: wtc → jbeich
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: needinfo?(wtc)
Priority: -- → P1
Resolution: --- → FIXED
Target Milestone: --- → 4.10.8
(Assignee)

Comment 8

4 years ago
(In reply to Wan-Teh Chang from comment #6)
> ::: nsprpub/pr/src/misc/prsystem.c
> @@ +300,3 @@
> >      uint64_t memSize;
> > +#else
> > +    unsigned long memSize;
> 
> I found some sample code on the Web that declares this variable as
> unsigned int. Which type is correct?

There's no correct type for HW_PHYSMEM. It was originally |int| on 4.4BSD and still is on OpenBSD.

FreeBSD casted off signedness and changed to long.
https://github.com/freebsd/freebsd/commit/c7c00ab (ca. FreeBSD 4.5)
https://github.com/freebsd/freebsd/commit/c3bdd66 (ca. FreeBSD 5.0)

NetBSD casted off signedness and added HW_PHYSMEM64.
https://github.com/jsonn/src/commit/35da98c

OpenBSD adapted HW_PHYSMEM64.
http://anoncvs.estpak.ee/cgi-bin/cgit/openbsd-src/commit/?id=c2e18ba

DragonFly inherited unsigned from FreeBSD 4.8 and later adapted long.
http://gitweb.dragonflybsd.org/dragonfly.git/commitdiff/39d69da
You need to log in before you can comment on or make changes to this bug.