On Thu, Nov 07 2019, Jeremie Courreges-Anglas <[email protected]> wrote:
> Back to this,
>
> On Tue, Nov 05 2019, Kurt Miller <[email protected]> wrote:
>> On Tue, 2019-11-05 at 09:17 +0100, Jan Beich wrote:
>>> Jeremie Courreges-Anglas <[email protected]> writes:
>>>
>>> >
>>> > ++#elif defined(HAVE_BSD_SYSCTL) && (defined(HW_MEMSIZE) ||
>>> > defined(HW_PHYSMEM64))
>>> > + int64_t physical_memory;
>>> HW_MEMSIZE and HW_PHYSMEM64 return uint64_t, not int64_t.
>
> Hrm, we document this as int64_t, even though the value is obviously not
> negative (that's the type used by sysctl_rdquad).
>
> NetBSD documents HW_PHYSMEM64 as "quad" without mentioning signedness,
> the underlying code in the kernel uses u_quad_t.
>
> Apple seems to document HW_MEMSIZE as an "integer" in its manpage, even
> though the kernel variable is an uint64_t.
>
>>> > ++#elif defined(HAVE_BSD_SYSCTL) && defined(HW_PHYSMEM))
>>> > ++ int physical_memory;
>>> HW_PHYSMEM returns u_long (unsigned long) on DragonFly and FreeBSD.
>
> I'll trust you on this, sadly... sysctl.3 says only "integer", and
> sys/sysctl.h says "int", which doesn't help much.
>
> https://svnweb.freebsd.org/base/head/sys/sys/sysctl.h
> https://www.freebsd.org/cgi/man.cgi?query=sysctl&apropos=0&sektion=3
>
> The whole situation is a mess. I guess the most reasonable approach
> using sysctl is what Jan proposed, use uint64_t for HW_PHYSMEM64 and
> HW_MEMSIZE, and fall back on HW_PHYSMEM + u_long on FreeBSD. I don't
> think signedness would cause any issue.
>
>>> int or signed long may upset -fsanitize=integer on 32-bit archs.
>>>
>>> Note, the code can be simplified via sysconf(3).
>
> Indeed. Initially I thought the sysctl hw.* code could be fixed; but now
> that I've rolled into this mud I think sysconf should be promoted as
> a simpler and more portable way to handle this, even with _SC_PHYS_PAGES
> not being defined by POSIX. The sysinfo code path used on Linux could
> also be deleted this way.
>
> This should be discussed with upstream, but seeing adoption on our side
> could help convince them. Jan, would FreeBSD be happy with this
> approach? Benoit, Klemens, Kurt: thoughts/oks?
>
> If people like this approach I can try to push it upstream, along with
> the ncpu diff.
Diff rebased on -current.
Index: Makefile
===================================================================
RCS file: /cvs/ports/devel/git/Makefile,v
retrieving revision 1.207
diff -u -p -r1.207 Makefile
--- Makefile 11 Nov 2019 10:12:32 -0000 1.207
+++ Makefile 11 Nov 2019 19:03:10 -0000
@@ -5,6 +5,7 @@ COMMENT-svn = subversion interoperabilit
COMMENT-x11 = graphical tools
V = 2.24.0
+REVISION = 0
DISTNAME = git-${V}
PKGNAME-main = ${DISTNAME}
PKGNAME-svn = git-svn-${V}
Index: patches/patch-builtin_gc_c
===================================================================
RCS file: /cvs/ports/devel/git/patches/patch-builtin_gc_c,v
retrieving revision 1.2
diff -u -p -r1.2 patch-builtin_gc_c
--- patches/patch-builtin_gc_c 11 Nov 2019 10:12:33 -0000 1.2
+++ patches/patch-builtin_gc_c 11 Nov 2019 19:03:10 -0000
@@ -1,38 +1,34 @@
$OpenBSD: patch-builtin_gc_c,v 1.2 2019/11/11 10:12:33 kn Exp $
-HW_PHYSMEM asks for an int, not an int64_t.
-Use HW_PHYSMEM64 to handle ram size > 2GB.
+Replace sysctl HW_PHYSMEM/MEMSIZE/whatever madness with sysconf.
Index: builtin/gc.c
--- builtin/gc.c.orig
+++ builtin/gc.c
-@@ -243,7 +243,7 @@ static uint64_t total_ram(void)
+@@ -244,20 +244,13 @@ static uint64_t total_ram(void)
if (!sysinfo(&si))
return si.totalram;
-#elif defined(HAVE_BSD_SYSCTL) && (defined(HW_MEMSIZE) || defined(HW_PHYSMEM))
-+#elif defined(HAVE_BSD_SYSCTL) && (defined(HW_MEMSIZE) ||
defined(HW_PHYSMEM64))
- int64_t physical_memory;
- int mib[2];
- size_t length;
-@@ -252,9 +252,19 @@ static uint64_t total_ram(void)
- # if defined(HW_MEMSIZE)
- mib[1] = HW_MEMSIZE;
- # else
+- int64_t physical_memory;
+- int mib[2];
+- size_t length;
++#elif defined(_SC_PHYS_PAGES) && defined(_SC_PAGESIZE)
++ long phys_pages, pagesize;
+
+- mib[0] = CTL_HW;
+-# if defined(HW_MEMSIZE)
+- mib[1] = HW_MEMSIZE;
+-# else
- mib[1] = HW_PHYSMEM;
-+ mib[1] = HW_PHYSMEM64;
- # endif
- length = sizeof(int64_t);
-+ if (!sysctl(mib, 2, &physical_memory, &length, NULL, 0))
-+ return physical_memory;
-+#elif defined(HAVE_BSD_SYSCTL) && defined(HW_PHYSMEM))
-+ int physical_memory;
-+ int mib[2];
-+ size_t length;
-+
-+ mib[0] = CTL_HW;
-+ mib[1] = HW_PHYSMEM;
-+ length = sizeof(int);
- if (!sysctl(mib, 2, &physical_memory, &length, NULL, 0))
- return physical_memory;
+-# endif
+- length = sizeof(int64_t);
+- if (!sysctl(mib, 2, &physical_memory, &length, NULL, 0))
+- return physical_memory;
++ phys_pages = sysconf(_SC_PHYS_PAGES);
++ pagesize = sysconf(_SC_PAGESIZE);
++ if (phys_pages != -1 && pagesize != -1)
++ return (uint64_t)phys_pages * (uint64_t)pagesize;
#elif defined(GIT_WINDOWS_NATIVE)
+ MEMORYSTATUSEX memInfo;
+
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
