On 2021/10/27 17:14, Martijn van Duren wrote:
> Trying to search for memory leaks in my new snmpd code I found some
> harmless, but annoying ones in system from SNMPv2-MIB.
>
> We call uname(3) every time (even if we don't even need info from
> that call) and ones set we save it until forever.
>
> Diff below calls uname(3) only a single time and if the variables
> aren't snmpd.conf I simply rebuild it every time.
I'm in two minds about this. While it's not something that will happen
very often, hostnames do get changed sometimes, and it would be nice to
have snmpd reflect the current name of the system rather than what it
was named when snmpd started.
> I also changed the default "system contact" to an empty string,
> since SNMPv2-MIB states:
> "If no contact information is known, the value is the zero-length
> string."
> I've basically never came across a system where there was both
> a smtp process accepting remote mail for the root-account and
> where the hostname were correctly resolving. If people are really
> attached to this value it's easy enough to set it, but I think
> it would be better to not tell unfounded assertions.
I'm certainly OK with that part.
> OK?
>
> martijn@
>
> Index: mib.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/mib.c,v
> retrieving revision 1.102
> diff -u -p -r1.102 mib.c
> --- mib.c 1 Sep 2021 15:54:40 -0000 1.102
> +++ mib.c 27 Oct 2021 16:14:15 -0000
> @@ -126,22 +126,25 @@ mib_getsys(struct oid *oid, struct ber_o
> {
> struct ber_oid sysoid = OID(MIB_SYSOID_DEFAULT);
> char *s = oid->o_data;
> + char buf[256];
> struct ber_oid *so = oid->o_data;
> - struct utsname u;
> + static struct utsname u;
> + int init = 0;
> long long ticks;
>
> - if (uname(&u) == -1)
> - return (-1);
> + if (!init) {
> + if (uname(&u) == -1)
> + return (-1);
> + init = 1;
> + }
>
> switch (oid->o_oid[OIDIDX_system]) {
> case 1:
> if (s == NULL) {
> - if (asprintf(&s, "%s %s %s %s %s",
> + snprintf(buf, sizeof(buf), "%s %s %s %s %s",
> u.sysname, u.nodename, u.release,
> - u.version, u.machine) == -1)
> - return (-1);
> - oid->o_data = s;
> - oid->o_val = strlen(s);
> + u.version, u.machine);
> + s = buf;
> }
> *elm = ober_add_string(*elm, s);
> break;
> @@ -157,21 +160,13 @@ mib_getsys(struct oid *oid, struct ber_o
> ober_set_header(*elm, BER_CLASS_APPLICATION, SNMP_T_TIMETICKS);
> break;
> case 4:
> - if (s == NULL) {
> - if (asprintf(&s, "root@%s", u.nodename) == -1)
> - return (-1);
> - oid->o_data = s;
> - oid->o_val = strlen(s);
> - }
> + if (s == NULL)
> + s = "";
> *elm = ober_add_string(*elm, s);
> break;
> case 5:
> - if (s == NULL) {
> - if ((s = strdup(u.nodename)) == NULL)
> - return (-1);
> - oid->o_data = s;
> - oid->o_val = strlen(s);
> - }
> + if (s == NULL)
> + s = u.nodename;
> *elm = ober_add_string(*elm, s);
> break;
> case 6:
> Index: snmpd.conf.5
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/snmpd.conf.5,v
> retrieving revision 1.58
> diff -u -p -r1.58 snmpd.conf.5
> --- snmpd.conf.5 2 Sep 2021 05:41:02 -0000 1.58
> +++ snmpd.conf.5 27 Oct 2021 16:14:15 -0000
> @@ -244,9 +244,6 @@ This is the default value.
> .It Ic system contact Ar string
> Specify the name or description of the system contact, typically a
> name or an email address.
> -The default value is
> -.Ar root@hostname
> -using the hostname of the local machine.
> .It Ic system description Ar string
> Specify a description of the local system.
> The default value is the operating system identification as printed by the
>
>