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. Index: Makefile =================================================================== RCS file: /cvs/ports/devel/git/Makefile,v retrieving revision 1.206 diff -u -p -r1.206 Makefile --- Makefile 3 Nov 2019 15:27:44 -0000 1.206 +++ Makefile 7 Nov 2019 14:47:18 -0000 @@ -5,7 +5,7 @@ COMMENT-svn = subversion interoperabilit COMMENT-x11 = graphical tools V = 2.23.0 -REVISION = 2 +REVISION = 3 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.1 diff -u -p -r1.1 patch-builtin_gc_c --- patches/patch-builtin_gc_c 3 Nov 2019 15:25:22 -0000 1.1 +++ patches/patch-builtin_gc_c 7 Nov 2019 14:47:18 -0000 @@ -1,38 +1,34 @@ $OpenBSD: patch-builtin_gc_c,v 1.1 2019/11/03 15:25:22 jca 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 -@@ -244,7 +244,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; -@@ -253,9 +253,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
