Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 14 May 2016 23:25:11 -0700
From:      Conrad Meyer <cem@FreeBSD.org>
To:        Garrett Cooper <ngie@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: svn commit: r299778 - head/usr.sbin/bsnmpd/tools/libbsnmptools
Message-ID:  <CAG6CVpXPXOLcS1EPR-uoeHxCvp8jJiuWNGS6ViZ0DtShuusUGA@mail.gmail.com>
In-Reply-To: <201605142322.u4ENMcxV083873@repo.freebsd.org>
References:  <201605142322.u4ENMcxV083873@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, May 14, 2016 at 4:22 PM, Garrett Cooper <ngie@freebsd.org> wrote:
> Author: ngie
> Date: Sat May 14 23:22:38 2016
> New Revision: 299778
> URL: https://svnweb.freebsd.org/changeset/base/299778
>
> Log:
>   Use a consistent errno save/restore pattern before running strtoul

This is begging to be reduced to a subroutine for the common expression.

Best,
Conrad


>
>   - Save errno
>   - Set errno to 0
>   - Call strtoul
>   - Test errno (optional, but many calls to strtoul did this afterwards)
>
>   Some of the code was setting errno = 0 after calling strtoul, not setting
>   errno = 0, or setting errno to saved_errno after the call, but before the
>   test. These all have unwanted behavioral side-effects, depending on the
>   initial value of errno and whether or not the input to strtoul was correct
>   or incorrect.
>
>   MFC after: 3 weeks
>   Sponsored by: EMC / Isilon Storage Division
>
> Modified:
>   head/usr.sbin/bsnmpd/tools/libbsnmptools/bsnmptc.c
>
> Modified: head/usr.sbin/bsnmpd/tools/libbsnmptools/bsnmptc.c
> ==============================================================================
> --- head/usr.sbin/bsnmpd/tools/libbsnmptools/bsnmptc.c  Sat May 14 23:22:19 2016        (r299777)
> +++ head/usr.sbin/bsnmpd/tools/libbsnmptools/bsnmptc.c  Sat May 14 23:22:38 2016        (r299778)
> @@ -364,6 +364,7 @@ snmp_date2asn_oid(char *str, struct asn_
>         /* 'MM-' */
>         ptr = endptr + 1;
>         saved_errno = errno;
> +       errno = 0;
>         v = strtoul(ptr, &endptr, 10);
>         if (errno != 0)
>                 goto error;
> @@ -377,6 +378,7 @@ snmp_date2asn_oid(char *str, struct asn_
>         /* 'DD,' */
>         ptr = endptr + 1;
>         saved_errno = errno;
> +       errno = 0;
>         v = strtoul(ptr, &endptr, 10);
>         if (errno != 0)
>                 goto error;
> @@ -390,6 +392,7 @@ snmp_date2asn_oid(char *str, struct asn_
>         /* 'HH:' */
>         ptr = endptr + 1;
>         saved_errno = errno;
> +       errno = 0;
>         v = strtoul(ptr, &endptr, 10);
>         if (errno != 0)
>                 goto error;
> @@ -403,6 +406,7 @@ snmp_date2asn_oid(char *str, struct asn_
>         /* 'MM:' */
>         ptr = endptr + 1;
>         saved_errno = errno;
> +       errno = 0;
>         v = strtoul(ptr, &endptr, 10);
>         if (errno != 0)
>                 goto error;
> @@ -416,6 +420,7 @@ snmp_date2asn_oid(char *str, struct asn_
>         /* 'SS.' */
>         ptr = endptr + 1;
>         saved_errno = errno;
> +       errno = 0;
>         v = strtoul(ptr, &endptr, 10);
>         if (errno != 0)
>                 goto error;
> @@ -429,6 +434,7 @@ snmp_date2asn_oid(char *str, struct asn_
>         /* 'M(mseconds),' */
>         ptr = endptr + 1;
>         saved_errno = errno;
> +       errno = 0;
>         v = strtoul(ptr, &endptr, 10);
>         if (errno != 0)
>                 goto error;
> @@ -454,6 +460,7 @@ snmp_date2asn_oid(char *str, struct asn_
>         /* 'HH:' */
>         ptr = endptr + 1;
>         saved_errno = errno;
> +       errno = 0;
>         v = strtoul(ptr, &endptr, 10);
>         if (errno != 0)
>                 goto error;
> @@ -467,6 +474,7 @@ snmp_date2asn_oid(char *str, struct asn_
>         /* 'MM' - last one - ignore endptr here. */
>         ptr = endptr + 1;
>         saved_errno = errno;
> +       errno = 0;
>         v = strtoul(ptr, &endptr, 10);
>         if (errno != 0)
>                 goto error;
> @@ -725,6 +733,7 @@ snmp_ntp_ts2asn_oid(char *str, struct as
>
>         ptr = str;
>         saved_errno = errno;
> +       errno = 0;
>         v = strtoul(ptr, &endptr, 10);
>         if (errno != 0 || (v / 1000) > 9) {
>                 warnx("Integer value %s not supported", str);
> @@ -749,6 +758,7 @@ snmp_ntp_ts2asn_oid(char *str, struct as
>
>         ptr = endptr + 1;
>         saved_errno = errno;
> +       errno = 0;
>         v = strtoul(ptr, &endptr, 10);
>         if (errno != 0 || (v / 1000) > 9) {
>                 warnx("Integer value %s not supported", str);
> @@ -776,6 +786,7 @@ parse_ntp_ts(struct snmp_value *sv, char
>         uint8_t ntp_ts[SNMP_NTP_TS_OCTETS];
>
>         saved_errno = errno;
> +       errno = 0;
>         v = strtoul(val, &endptr, 10);
>         if (errno != 0 || (v / 1000) > 9) {
>                 errno = saved_errno;
> @@ -797,6 +808,7 @@ parse_ntp_ts(struct snmp_value *sv, char
>         val = endptr + 1;
>
>         saved_errno = errno;
> +       errno = 0;
>         v = strtoul(val, &endptr, 10);
>         if (errno != 0 || (v / 1000) > 9) {
>                 errno = saved_errno;
> @@ -879,8 +891,8 @@ snmp_bridgeid2oct(char *str, struct asn_
>         ptr = str;
>         /* Read the priority. */
>         saved_errno = errno;
> -       v = strtoul(ptr, &endptr, 10);
>         errno = 0;
> +       v = strtoul(ptr, &endptr, 10);
>
>         if (v > SNMP_MAX_BRIDGE_PRIORITY || errno != 0 || *endptr != '.') {
>                 errno = saved_errno;
> @@ -897,6 +909,7 @@ snmp_bridgeid2oct(char *str, struct asn_
>         ptr = endptr + 1;
>         for (i = 0; i < 5; i++) {
>                 saved_errno = errno;
> +               errno = 0;
>                 v = strtoul(ptr, &endptr, 16);
>                 errno = saved_errno;
>                 if (v > 0xff) {
> @@ -914,6 +927,7 @@ snmp_bridgeid2oct(char *str, struct asn_
>
>         /* The last one - don't check the ending char here. */
>         saved_errno = errno;
> +       errno = 0;
>         v = strtoul(ptr, &endptr, 16);
>         errno = saved_errno;
>         if (v > 0xff) {
> @@ -938,7 +952,6 @@ parse_bridge_id(struct snmp_value *sv, c
>         saved_errno = errno;
>         errno = 0;
>         v = strtoul(string, &endptr, 10);
> -       errno = saved_errno;
>
>         if (v > SNMP_MAX_BRIDGE_PRIORITY || errno != 0 || *endptr != '.') {
>                 errno = saved_errno;
> @@ -1026,8 +1039,8 @@ snmp_bport_id2oct(char *str, struct asn_
>         ptr = str;
>         /* Read the priority. */
>         saved_errno = errno;
> -       v = strtoul(ptr, &endptr, 10);
>         errno = 0;
> +       v = strtoul(ptr, &endptr, 10);
>
>         if (v > SNMP_MAX_BPORT_PRIORITY || errno != 0 || *endptr != '.') {
>                 errno = saved_errno;
> @@ -1039,6 +1052,7 @@ snmp_bport_id2oct(char *str, struct asn_
>                 return (NULL);
>
>         saved_errno = errno;
> +       errno = 0;
>         v = strtoul(ptr, &endptr, 16);
>         errno = saved_errno;
>
> @@ -1065,7 +1079,6 @@ parse_bport_id(struct snmp_value *value,
>         saved_errno = errno;
>         errno = 0;
>         v = strtoul(string, &endptr, 10);
> -       errno = saved_errno;
>
>         if (v > SNMP_MAX_BPORT_PRIORITY || errno != 0 || *endptr != '.') {
>                 errno = saved_errno;
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG6CVpXPXOLcS1EPR-uoeHxCvp8jJiuWNGS6ViZ0DtShuusUGA>