pkgsrc-WIP-changes archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Improve stability and portability, and overall simplify the code:



Module Name:	pkgsrc-wip
Committed By:	Paolo Vincenzo Olivo <vms%retrobsd.ddns.net@localhost>
Pushed By:	vms
Date:		Sat Dec 3 00:36:00 2022 +0100
Changeset:	60ca69d8fdaf3359e9c3858a7df75a565570cf6d

Modified Files:
	bsdfetch/Makefile
	bsdfetch/distinfo
Added Files:
	bsdfetch/patches/patch-bsdfetch.c
	bsdfetch/patches/patch-sysctlbyname.c

Log Message:
Improve stability and portability, and overall simplify the code:

* Improve shell and username detection (rely on getpwuid if environment
  is null).

* Use portable `sysconf(_SC_NPROCESSORS_*)' instead of
  `sysctlbyname("hw.ncpu*")' to detected numbers of cores online.

* Significantly simplify CPU model detection on NetBSD,
  using `machdep.cpu_brand' sysctl variable instead of /proc/cpuinfo.
  This adds potential support for macOS.

* Simplify and pack together OS-dependent conditionals in
  get_packages().

* Drop libpkg(3) on FreeBSD, as the performance gain is probably
  negligible, and switch to a common popen() function.

* Simplify getting package count in get_packages and save a couple of
  processes.

* Use portable kern.boottime sysctl variable in place of CLOCK_UPTIME
  inside get_uptime().

* Use portable uname(3) instead of OS-dependet sysctl variables to get
  machine architecture in get_arch().

* In get_memory, replace the various OS-dependent
  sysctlbyname("hw.*mem*") with portable sysconf() functions.

* Fix unfree malloc() in get_loadavg().

* Fix hostname[15] overflow by gethostname() in get_hostname().

* Add a global `buf[BUFSIZ]' and use this in every function instead of
  a (potentially) small local buffer in each.

* Replace sprintf() with snprintf() wherever possible.

* Add new required headers.

To see a diff of this commit:
https://wip.pkgsrc.org/cgi-bin/gitweb.cgi?p=pkgsrc-wip.git;a=commitdiff;h=60ca69d8fdaf3359e9c3858a7df75a565570cf6d

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

diffstat:
 bsdfetch/Makefile                     |   6 +
 bsdfetch/distinfo                     |   2 +
 bsdfetch/patches/patch-bsdfetch.c     | 370 ++++++++++++++++++++++++++++++++++
 bsdfetch/patches/patch-sysctlbyname.c |  15 ++
 4 files changed, 393 insertions(+)

diffs:
diff --git a/bsdfetch/Makefile b/bsdfetch/Makefile
index e18f03ccb2..5a608e9d2a 100644
--- a/bsdfetch/Makefile
+++ b/bsdfetch/Makefile
@@ -15,6 +15,12 @@ ONLY_FOR_PLATFORM+=	NetBSD-*-*
 ONLY_FOR_PLATFORM+=	OpenBSD-*-*
 ONLY_FOR_PLATFORM+=	DragonFly-*-*
 
+CFLAGS+=	-Wall -Wunused -Wextra
+CFLAGS+=	-Wshadow -pedantic
+
+MAKE_FLAGS+=	CC=${CC:Q}
+MAKE_FLAGS+=	CFLAGS=${CFLAGS:Q}
+
 INSTALLATION_DIRS+=	bin
 
 do-install:
diff --git a/bsdfetch/distinfo b/bsdfetch/distinfo
index a0c2d0a68f..a5994e2411 100644
--- a/bsdfetch/distinfo
+++ b/bsdfetch/distinfo
@@ -4,3 +4,5 @@ BLAKE2s (bsdfetch-0.9-9840c3153fcf49150ebcee94f527f4ebcc0b8c1b.tar.gz) = ee4bd56
 SHA512 (bsdfetch-0.9-9840c3153fcf49150ebcee94f527f4ebcc0b8c1b.tar.gz) = b62d956f25eacc2426743c09523e89ade002fadd44445aa5c2628425b10d45d0133354be765af17fac78a936436197a8de5b02efd9c670dcdcf9513f0b744bb3
 Size (bsdfetch-0.9-9840c3153fcf49150ebcee94f527f4ebcc0b8c1b.tar.gz) = 6794 bytes
 SHA1 (patch-Makefile) = 62812c29b42d05ad9a9b1b9cc263ec309a7a4771
+SHA1 (patch-bsdfetch.c) = 703897e4c34b95327d9db8d7515231bd73f2b893
+SHA1 (patch-sysctlbyname.c) = d00d3466cc3be82c7cf01ecfb6b5b12099adcc44
diff --git a/bsdfetch/patches/patch-bsdfetch.c b/bsdfetch/patches/patch-bsdfetch.c
new file mode 100644
index 0000000000..f008426dd7
--- /dev/null
+++ b/bsdfetch/patches/patch-bsdfetch.c
@@ -0,0 +1,370 @@
+$NetBSD$
+
+Stability and portability fixes.
+
+--- bsdfetch.c.orig	2022-11-30 09:00:21.000000000 +0000
++++ bsdfetch.c
+@@ -18,6 +18,8 @@
+ #include <stdlib.h>
+ #include <string.h>
+ #include <unistd.h>
++#include <limits.h>
++#include <pwd.h>
+ #include <sys/types.h>
+ #include <sys/sysctl.h>
+ #include <sys/utsname.h>
+@@ -45,20 +47,11 @@
+ 
+ #define _SILENT (void)
+ 
+-#define LIBPKGSO "/lib/libpkg.so"
+-
+-struct pkgdb;
+-struct pkgdb_it;
+-typedef int (*pkg_init_fp)(const char *, const char*);
+-typedef void (*pkg_shutdown_fp)(void);
+-typedef int (*pkgdb_open_fp)(struct pkgdb **db, int type);
+-typedef void (*pkgdb_close_fp)(struct pkgdb *db);
+-typedef struct pkgdb_it *(*pkgdb_query_fp)(struct pkgdb *db,
+-	const char *pattern, int type);
+-typedef int (*pkgdb_it_count_fp)(struct pkgdb_it *);
+-
+ typedef unsigned int uint;
+ 
++char tmp[100] = {0};
++char buf[100] = {0};
++
+ int color_flag = 1;
+ 
+ static void die(int err_num, int line);
+@@ -95,55 +88,70 @@ static void show(const char *entry, cons
+ }
+ 
+ static void get_shell() {
+-	show("Shell", getenv("SHELL"));
++	char *sh;
++	char *lsh;
++	uid_t uid = geteuid();
++	struct passwd *pw = getpwuid(uid);
++	const char ch = '/';
++
++	if (getenv("SHELL")) {
++		sh = getenv("SHELL");
++	} else {
++		if (!pw)
++			die(errno, __LINE__);
++		sh = pw->pw_shell;
++	}
++
++	if ((lsh = strrchr(sh, ch)))
++		sh = lsh + 1;
++
++	show("Shell", sh);
+ }
+ 
+ static void get_user() {
+-	show("User", getenv("USER"));
++	char *user;
++	uid_t uid = geteuid();
++	struct passwd *pw = getpwuid(uid);
++
++	if (getenv("USER")) {
++		user = getenv("USER");
++	} else {
++		if (!pw)
++			die(errno, __LINE__);
++		user = pw->pw_name;
++		}
++
++	show("User", user);
+ }
+ 
+ static void get_cpu() {
+-	size_t num_cpu_size = 0;
+-	uint num_cpu = 0;
++	size_t cpu_type_size = 0;
++	uint ncpu = 0;
++	uint ncpu_max = 0;
+ 	char cpu_type[200] = {0};
+-	char tmp[100] = {0};
+ 
+-	num_cpu_size = sizeof(num_cpu);
+-
+-	if(sysctlbyname("hw.ncpu", &num_cpu, &num_cpu_size, NULL, 0) == -1)
+-		die(errno, __LINE__);
+-
+-#if defined(__NetBSD__)
+-	FILE *fc = NULL;
+-
+-	fc = popen("awk 'BEGIN{FS=\":\"} /model name/ { print $2; exit }' "
+-                   "/proc/cpuinfo | sed -e 's/ @//' -e 's/^ *//g' -e 's/ *$//g' "
+-                   "| head -1 | tr -d '\\n'",
+-                   "r");
+-	if (fc == NULL)
++	ncpu = sysconf(_SC_NPROCESSORS_ONLN);
++	ncpu_max = sysconf(_SC_NPROCESSORS_CONF);
++	if (ncpu_max <= 0 || ncpu <=0)
+ 		die(errno, __LINE__);
+ 
+-	fgets(cpu_type, sizeof(cpu_type), fc);
+-	pclose(fc);
+-#else
+-	size_t cpu_type_size = 0;
+ 	cpu_type_size = sizeof(char) * 200;
+-	if(sysctlbyname("hw.model", &cpu_type, &cpu_type_size, NULL, 0) == -1)
+-		die(errno, __LINE__);
+-#endif
++	if(sysctlbyname("machdep.cpu_brand", &cpu_type, &cpu_type_size, NULL, 0) == -1)
++		if(sysctlbyname("hw.model", &cpu_type, &cpu_type_size, NULL, 0) == -1)
++			die(errno, __LINE__);
++
+ 	show("CPU", cpu_type);
+ 
+-	_SILENT sprintf(tmp, "%d", num_cpu);
++	_SILENT sprintf(tmp, "%d of %d processors online", ncpu, ncpu_max);
+ 
+ 	show("Cores", tmp);
+ 
+ #if defined(__FreeBSD__) || defined(__MidnightBSD__) || defined(__DragonFly__)
+ 	for(uint i = 0; i < num_cpu; i++) {
+ 		size_t temperature_size = 0;
+-		char buf[100] = {0};
+ 		int temperature = 0;
+ 
+-		sprintf(buf, "dev.cpu.%d.temperature", i);
++		snprintf(buf, "dev.cpu.%d.temperature", i);
+ 
+ 		temperature_size = sizeof(buf);
+ 		if(sysctlbyname(buf, &temperature, &temperature_size, NULL, 0) == -1)
+@@ -171,17 +179,14 @@ static void get_cpu() {
+ 	if(sysctl(mib, 5, &sensors, &size, NULL, 0) < 0)
+ 		return;
+ 
+-	_SILENT sprintf(temp, "%d °C", (int)((float)(sensors.value - 273150000) / 1E6));
++	_SILENT snprintf(temp, "%d °C", (int)((float)(sensors.value - 273150000) / 1E6));
+ 
+ 	show("CPU Temp", temp);
+ #endif
+ }
+ 
+ static void get_loadavg() {
+-	char tmp[20] = {0};
+-	double *lavg = NULL;
+-
+-	lavg = malloc(sizeof(double) * 3);
++	double lavg[3];
+ 
+ 	(void)getloadavg(lavg, -1);
+ 
+@@ -191,120 +196,45 @@ static void get_loadavg() {
+ }
+ 
+ static void get_packages() {
+-#if defined(__MidnightBSD__)
+-	FILE *f = NULL;
+-	char buf[10] = {0};
+-
+-	/*
+-	  It might be better to use the mport stats functionality long term, but this
+-	  avoids parsing.
+-	*/
+-	f = popen("/usr/sbin/mport list | wc -l | tr -d \"\n \"", "r");
+-	if(f == NULL)
+-		die(errno, __LINE__);
+-
+-	fgets(buf, sizeof(buf), f);
+-	pclose(f);
+-
+-	show("Packages", buf);
+-#elif defined(__FreeBSD__)
+-	int numpkg = 0;
+-	void *libhdl = 0;
+-	struct pkgdb *pdb = 0;
+-	char buf[256];
+-	size_t basesz = sizeof buf;
+-
+-	if (sysctlbyname("user.localbase", buf, &basesz, NULL, 0) < 0)
+-	    goto done;
+-	if (sizeof buf - basesz < sizeof LIBPKGSO - 1)
+-	    goto done;
+-	memcpy(buf + basesz - 1, LIBPKGSO, sizeof LIBPKGSO);
+-
+-	if (!(libhdl = dlopen(buf, RTLD_LAZY))) goto done;
+-	pkg_init_fp p_init = (pkg_init_fp)dlsym(libhdl, "pkg_init");
+-	if (!p_init) goto done;
+-	pkg_shutdown_fp p_shutdown = (pkg_shutdown_fp)dlsym(
+-		libhdl, "pkg_shutdown");
+-	if (!p_shutdown) goto done;
+-	pkgdb_open_fp pdb_open = (pkgdb_open_fp)dlsym(libhdl, "pkgdb_open");
+-	if (!pdb_open) goto done;
+-	pkgdb_close_fp pdb_close = (pkgdb_close_fp)dlsym(libhdl, "pkgdb_close");
+-	if (!pdb_close) goto done;
+-	pkgdb_query_fp pdb_query = (pkgdb_query_fp)dlsym(libhdl, "pkgdb_query");
+-	if (!pdb_query) goto done;
+-	pkgdb_it_count_fp pdb_it_count = (pkgdb_it_count_fp)dlsym(
+-		libhdl, "pkgdb_it_count");
+-	if (!pdb_it_count) goto done;
+-
+-	if (p_init("/", 0) != 0) goto done;
+-	if (pdb_open(&pdb, 0) != 0) goto done;
+-	struct pkgdb_it *it = pdb_query(pdb, 0, 0);
+-	numpkg = pdb_it_count(it);
+-
+-done:
+-	if (pdb) pdb_close(pdb);
+-	if (libhdl)
+-	{
+-	    if (p_shutdown) p_shutdown();
+-	    dlclose(libhdl);
+-	}
+-	sprintf(buf, "%d", numpkg);
+-	show("Packages", buf);
+-#elif defined(__OpenBSD__) || defined(__NetBSD__)
+ 	FILE *f = NULL;
+-	char buf[10] = {0};
++	size_t npkg = 0;
+ 
+-	/*
+-		This is a little hacky for the moment. I don't
+-		see another good solution to get the package
+-		count on OpenBSD.
+-		Still, this works fine.
+-	*/
+-	f = popen("/usr/sbin/pkg_info | wc -l | tr -d \"\n \"", "r");
++#if defined(__OpenBSD__) || defined(__NetBSD__)
++	f = popen("/usr/sbin/pkg_info", "r");
++#elif defined(__FreeBSD__) || ( __DragonFly__)
++	f = popen("/usr/sbin/pkg info", "r");
++#elif defined( __MidnightBSD__)
++	f = popen("/usr/sbin/mport list", "r");
++#else
++	fprintf(stderr, "Could not determine BSD variant\n");
++	die(errno, __LINE__);
++#endif
+ 	if(f == NULL)
+ 		die(errno, __LINE__);
+ 
+-	fgets(buf, sizeof(buf), f);
+-	pclose(f);
+-
+-	show("Packages", buf);
+-#elif defined( __DragonFly__)
+-	char buf[10] ={0};
+-	FILE *fp = NULL;
+-
+-	/**
+-	 * Despite being a fork of FreeBSD 4.8, DragonFly doesn't share
+-	 * same API level access. Here `pkg list` just list all the packages
+-	 * from the local database.
+-	*/
+-	fp = popen("pkg list | wc -l | tr -d \"\n \"", "r");
+-	if (fp == NULL)
++	while (fgets(buf, sizeof buf, f) != NULL)
++		if (strchr(buf, '\n') != NULL)
++			npkg++;
++	if (pclose(f) != 0)
+ 		die(errno, __LINE__);
+ 
+-	fgets(buf, sizeof(buf), fp);
+-	pclose(fp);
+-
++	snprintf(buf, sizeof buf, "%zu", npkg);
+ 	show("Packages", buf);
+-#endif
+ }
+ 
+ static void get_uptime() {
+-	char buf[100] = {0};
+ 	int up = 0;
+ 	int ret = 0;
+ 	int days = 0;
+ 	int hours = 0;
+ 	int minutes = 0;
+ 	struct timespec t;
++	size_t tsz = sizeof t;
+ 
+-#ifndef CLOCK_UPTIME
+-#define CLOCK_UPTIME CLOCK_MONOTONIC
+-#endif
+-	ret = clock_gettime(CLOCK_UPTIME, &t);
+-	if(ret == -1)
++	if ((ret = sysctlbyname("kern.boottime", &t, &tsz, NULL, 0)) == -1)
+ 		die(errno, __LINE__);
+ 
+-	up = t.tv_sec;
++	up = time(NULL) - t.tv_sec + 30;
+ 	days = up / 86400;
+ 	up %= 86400;
+ 	hours = up / 3600;
+@@ -317,25 +247,20 @@ static void get_uptime() {
+ }
+ 
+ static void get_memory() {
+-	unsigned long long buf = 0;
++	unsigned long long buff = 0;
+ 	unsigned long long mem = 0;
+-	char tmp[50] = {0};
+-	size_t buf_size = 0;
+-
+-	buf_size = sizeof(buf);
++	const long pgsize = sysconf(_SC_PAGESIZE);
++	const long pages = sysconf(_SC_PHYS_PAGES);
+ 
+-#if defined(__FreeBSD__) || defined(__MidnightBSD__)
+-	if(sysctlbyname("hw.realmem", &buf, &buf_size, NULL, 0) == -1)
+-		die(errno, __LINE__);
+-#elif defined(__OpenBSD__) || defined(__DragonFly__)
+-	if(sysctlbyname("hw.physmem", &buf, &buf_size, NULL, 0) == -1)
++	if (pgsize == -1 || pages == -1)
+ 		die(errno, __LINE__);
+-#elif defined(__NetBSD__)
+-	if(sysctlbyname("hw.physmem64", &buf, &buf_size, NULL, 0) == -1)
+-		die(errno, __LINE__);
+-#endif
++	else
++		buff = (uint64_t)pgsize * (uint64_t)pages;
+ 
+-	mem = buf / 1048576;
++	if (buff <= 0)
++		die(errno, __LINE__);
++	else
++		mem = buff / 1048576;
+ 
+ 	_SILENT sprintf(tmp, "%llu MB", mem);
+ 
+@@ -344,9 +269,9 @@ static void get_memory() {
+ 
+ static void get_hostname() {
+ 	long host_size_max = 0;
+-	char hostname[15] = {0};
++	char hostname[_POSIX_HOST_NAME_MAX + 1];
+ 
+-	host_size_max = sysconf(_SC_HOST_NAME_MAX);
++	host_size_max = sysconf(_SC_HOST_NAME_MAX) + 1;
+ 	if(gethostname(hostname, host_size_max) == -1)
+ 		die(errno, __LINE__);
+ 
+@@ -354,20 +279,12 @@ static void get_hostname() {
+ }
+ 
+ static void get_arch() {
+-	char buf[20] = {0};
+-	size_t buf_size = 0;
+-
+-	buf_size = sizeof(buf);
++	struct utsname un;
+ 
+-#if defined(__FreeBSD__) || defined(__MidnightBSD__) || defined(__DragonFly__)
+-	if(sysctlbyname("hw.machine_arch", &buf, &buf_size, NULL, 0) == -1)
+-		die(errno, __LINE__);
+-#elif defined(__OpenBSD__) || defined(__NetBSD__)
+-	if(sysctlbyname("hw.machine", &buf, &buf_size, NULL, 0) == -1)
++	if(uname(&un))
+ 		die(errno, __LINE__);
+-#endif
+ 
+-	show("Arch", buf);
++	show("Arch", un.machine);
+ }
+ 
+ static void get_sysinfo() {
diff --git a/bsdfetch/patches/patch-sysctlbyname.c b/bsdfetch/patches/patch-sysctlbyname.c
new file mode 100644
index 0000000000..a441fd8f88
--- /dev/null
+++ b/bsdfetch/patches/patch-sysctlbyname.c
@@ -0,0 +1,15 @@
+$NetBSD$
+
+Simplify checking for sizeof sysctl names. 
+
+--- sysctlbyname.c.orig	2022-11-30 09:00:21.000000000 +0000
++++ sysctlbyname.c
+@@ -28,7 +28,7 @@ sysctlbyname(const char *name, void *old
+ {
+ 	int i, mib[2];
+ 
+-	for (i = 0; i < (int) (sizeof(sysctlnames) / sizeof(sysctlnames[0])); i++) {
++	for (i = 0; sysctlnames[i].name != NULL; i++) {
+ 		if (!strcmp(name, sysctlnames[i].name)) {
+ 			mib[0] = sysctlnames[i].mib0;
+ 			mib[1] = sysctlnames[i].mib1;


Home | Main Index | Thread Index | Old Index