782124 - Implement PR_GetPhysicalMemorySize on FreeBSD

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P1)

Comment 1

[Jan Beich]

Attachment: reuse code for (Net|Open)BSD

Comment 2

[Jan Beich]

Attachment: reuse code for other BSDs, v2

Now it should work on 32bit systems. Sorry.

Comment 3

[Jan Beich]

Attachment: 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

Comment 4

[Seth Fowler [:seth] [:s2h]]

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?

Comment 5

[Jan Beich]

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

typo check: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=f48f6aff3ee5

Also tested within FreeBSD ports and DragonFly DPorts.

https://svnweb.freebsd.org/ports/head/devel/nspr/files/patch-bug782124?view=markup
https://github.com/DragonFlyBSD/DPorts/blob/master/devel/nspr/files/patch-bug782124

Comment 6

[Wan-Teh Chang]

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?

Comment 7

[Wan-Teh Chang]

This fix will be in the upcoming NSPR 4.10.8 release.

Comment 8

[Jan Beich]

(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