ping
On Fri, 2021-10-01 at 16:42 +0200, Martijn van Duren wrote:
> Right now ober_oid_cmp has inverted logic compared to other *cmp
> functions. Specifically values greater than 0 are returned when
> a is smaller than b.
> Additionally, the value of 2 is only used b is a child of a, but
> -1 is returned when a is a child of b.
>
> This makes it harder than needed to use it with say the RB_*
> family of functions, as well as wanting to test between the
> parent-child relationship between two value.
>
> Both these issues are not present in ax_oid_cmp and I would like
> to copy the logic over from there.
>
> Manpage bits mostly taken from strcmp(3)
>
> I did a full scan of our tree and I could only find this function in
> snmp(1) and snmpd(8) and patches for those are attaches as well.
> No known 3rd party software is using this code.
>
> Regress for libagentx, snmp and snmp pass.
>
> OK for the next cycle?
>
> martijn@
>
> Index: lib/libutil/ber.c
> ===================================================================
> RCS file: /cvs/src/lib/libutil/ber.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 ber.c
> --- lib/libutil/ber.c 29 Aug 2021 13:27:11 -0000 1.22
> +++ lib/libutil/ber.c 1 Oct 2021 14:36:34 -0000
> @@ -473,26 +473,21 @@ ober_string2oid(const char *oidstr, stru
> int
> ober_oid_cmp(struct ber_oid *a, struct ber_oid *b)
> {
> - size_t i;
> - for (i = 0; i < a->bo_n && i < b->bo_n; i++) {
> - if (a->bo_id[i] == b->bo_id[i])
> - continue;
> - else if (a->bo_id[i] < b->bo_id[i]) {
> - /* b is a successor of a */
> - return (1);
> - } else {
> - /* b is a predecessor of a */
> + size_t i, min;
> +
> + min = a->bo_n < b->bo_n ? a->bo_n : b->bo_n;
> + for (i = 0; i < min; i++) {
> + if (a->bo_id[i] < b->bo_id[i])
> return (-1);
> - }
> + if (a->bo_id[i] > b->bo_id[i])
> + return (1);
> }
> - /* b is larger, but a child of a */
> + /* a is parent of b */
> if (a->bo_n < b->bo_n)
> - return (2);
> - /* b is a predecessor of a */
> + return (-2);
> + /* a is child of b */
> if (a->bo_n > b->bo_n)
> - return -1;
> -
> - /* b and a are identical */
> + return 2;
> return (0);
> }
>
> Index: lib/libutil/ober_oid_cmp.3
> ===================================================================
> RCS file: /cvs/src/lib/libutil/ober_oid_cmp.3,v
> retrieving revision 1.4
> diff -u -p -r1.4 ober_oid_cmp.3
> --- lib/libutil/ober_oid_cmp.3 12 Mar 2021 07:24:49 -0000 1.4
> +++ lib/libutil/ober_oid_cmp.3 1 Oct 2021 14:36:34 -0000
> @@ -66,26 +66,17 @@ returns the number of bytes written or 0
> returns 0 on success or -1 on failure.
> .Pp
> .Fn ober_oid_cmp
> -returns 0 when oids
> +returns an integer greater than, equal to, or less than 0, according to
> whether
> +the oid
> +.Fa a
> +is greater than, equal to, or less than the oid
> +.Fa b .
> +If the shortest length from
> .Fa a
> and
> .Fa b
> -are identical.
> -If
> -.Fa b
> -is a successor of
> -.Fa a ,
> -1 is returned.
> -If
> -.Fa b
> -is a predecessor of
> -.Fa a ,
> --1 is returned.
> -If
> -.Fa b
> -is larger, but a child of
> -.Fa a ,
> -2 is returned.
> +matches
> +the weight of the integer is 2, else it is 1.
> .Sh SEE ALSO
> .Xr ober_add_string 3 ,
> .Xr ober_get_string 3 ,
> Index: usr.sbin/snmpd/traphandler.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/traphandler.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 traphandler.c
> --- usr.sbin/snmpd/traphandler.c 22 Feb 2021 11:31:09 -0000 1.21
> +++ usr.sbin/snmpd/traphandler.c 1 Oct 2021 14:36:34 -0000
> @@ -440,7 +440,7 @@ trapcmd_cmp(struct trapcmd *cmd1, struct
> {
> int ret;
>
> - ret = ober_oid_cmp(cmd2->cmd_oid, cmd1->cmd_oid);
> + ret = ober_oid_cmp(cmd1->cmd_oid, cmd2->cmd_oid);
> switch (ret) {
> case 2:
> /* cmd1 is a child of cmd2 */
> Index: usr.bin/snmp/smi.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/snmp/smi.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 smi.c
> --- usr.bin/snmp/smi.c 4 Jan 2021 08:00:29 -0000 1.14
> +++ usr.bin/snmp/smi.c 1 Oct 2021 14:36:34 -0000
> @@ -506,7 +506,7 @@ smi_string2oid(const char *oidstr, struc
> *p++ = '\0';
> if ((oid = smi_findkey(sp)) != NULL) {
> bcopy(&oid->o_id, &ko, sizeof(ko));
> - if (o->bo_n && ober_oid_cmp(o, &ko) != 2)
> + if (o->bo_n && ober_oid_cmp(&ko, o) != 2)
> return (-1);
> bcopy(&ko, o, sizeof(*o));
> errstr = NULL;
> Index: usr.bin/snmp/snmpc.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/snmp/snmpc.c,v
> retrieving revision 1.37
> diff -u -p -r1.37 snmpc.c
> --- usr.bin/snmp/snmpc.c 11 Aug 2021 18:53:45 -0000 1.37
> +++ usr.bin/snmp/snmpc.c 1 Oct 2021 14:36:34 -0000
> @@ -640,7 +640,7 @@ snmpc_walk(int argc, char *argv[])
> }
> while (1) {
> for (i = 0; i < walk_skip_len; i++) {
> - skip_cmp = ober_oid_cmp(&(walk_skip[i]), &noid);
> + skip_cmp = ober_oid_cmp(&noid, &(walk_skip[i]));
> if (skip_cmp == 0 || skip_cmp == 2) {
> bcopy(&(walk_skip[i]), &noid, sizeof(noid));
> noid.bo_id[noid.bo_n -1]++;
> @@ -673,19 +673,19 @@ snmpc_walk(int argc, char *argv[])
> value->be_type == BER_TYPE_EOC)
> break;
> for (i = 0; i < walk_skip_len; i++) {
> - skip_cmp = ober_oid_cmp(&(walk_skip[i]), &noid);
> + skip_cmp = ober_oid_cmp(&noid, &(walk_skip[i]));
> if (skip_cmp == 0 || skip_cmp == 2)
> break;
> }
> if (i < walk_skip_len)
> continue;
> - prev_cmp = ober_oid_cmp(&loid, &noid);
> + prev_cmp = ober_oid_cmp(&noid, &loid);
> if (walk_check_increase && prev_cmp == -1)
> errx(1, "OID not increasing");
> - if (prev_cmp == 0 || ober_oid_cmp(&oid, &noid) != 2)
> + if (prev_cmp == 0 || ober_oid_cmp(&noid, &oid) != 2)
> break;
> if (walk_end.bo_n != 0 &&
> - ober_oid_cmp(&walk_end, &noid) != -1)
> + ober_oid_cmp(&noid, &walk_end) != -1)
> break;
>
> if (!snmpc_print(varbind))
> @@ -899,7 +899,7 @@ snmpc_df(int argc, char *argv[])
> for (; varbind != NULL; varbind = varbind->be_next) {
> if (ober_scanf_elements(varbind, "{os", &oid,
> &string) == -1 ||
> - ober_oid_cmp(&descroid, &oid) != 2)
> + ober_oid_cmp(&oid, &descroid) != 2)
> break;
> rows++;
> }
> @@ -913,7 +913,7 @@ snmpc_df(int argc, char *argv[])
> rows--;
> continue;
> }
> - if (ober_oid_cmp(&descroid, &oid) != 2)
> + if (ober_oid_cmp(&oid, &descroid) != 2)
> break;
> df[i].index = oid.bo_id[oid.bo_n - 1];
> if ((df[i].descr = smi_print_element(&oid, elm, 0,
>
