On Wed, Aug 14, 2019 at 02:37:14PM +0200, Martijn van Duren wrote:
> This one stumped me for longer than I'd like to admit.
> The problem I faced was that I expected
> ber_scanf_elements(..., "{Se{", ...) to work if "e" is an nstring.
> This was not the case because "e" doesn't increment ber and { is
> tested against the nstring.
>
> Because I can see value in having it this way (retrieve the value and
> element in the same run) I reckon we can keep it this way.
> If you'd want to only have "e" you can "eS".
>
> Patch one documents this behaviour.
>
> If we rather have this behaviour changed, in that it always increments
> that's patch two. This can be done safely:
> $ grep -lRF '#include <ber.h>' *bin | while read file; do grep -H
> ber_scanf_elements $file; done
> usr.bin/snmp/snmpc.c: (void) ber_scanf_elements(pdu, "{Sdd{e",
> &errorstatus, &errorindex,
> usr.bin/snmp/snmpc.c: (void) ber_scanf_elements(pdu, "{Sdd{e",
> &errorstatus,
> usr.bin/snmp/snmpc.c: (void) ber_scanf_elements(pdu, "{Sdd{e",
> &errorstatus,
> usr.bin/snmp/snmpc.c: (void) ber_scanf_elements(varbind,
> "{oe}", &noid,
> usr.bin/snmp/snmpc.c: (void) ber_scanf_elements(pdu, "{Sdd{e",
> &errorstatus,
> usr.sbin/snmpd/traphandler.c: if (ber_scanf_elements(*req, "{dSe", &vers,
> &elm) == -1)
> usr.sbin/snmpd/traphandler.c: if (ber_scanf_elements(elm, "{oSddd",
> usr.sbin/snmpd/traphandler.c: if (ber_scanf_elements(elm,
> "{SSS{e}}", &elm) == -1 ||
> usr.sbin/snmpd/traphandler.c: ber_scanf_elements(elm,
> "{Sd}{So}",
> usr.sbin/snmpd/traphandler.c: if (ber_scanf_elements(iter, "{oe}",
> &oid, &elm) == -1)
> usr.sbin/snmpctl/snmpclient.c: if (ber_scanf_elements(resp, "{SS{SSSS{e}}",
> &s) != 0)
> usr.sbin/snmpctl/snmpclient.c: if (ber_scanf_elements(s, "{oe}",
> &sc->sc_oid, &e) != 0)
> usr.sbin/snmpctl/snmpclient.c: if (ber_scanf_elements(resp, "{is{i", &ver,
> &comn, &id) != 0)
> Where "e" is always the last element checked.
> Using "}" just raises the level, but doesn't check current ber to see if
> it's the final element.
>
> OK for 1?
Ok claudio@
> Good motivation for 2?
In my opinion it would make sense to eat the element in 'e' like all other
data retrivals do. It feels like this is easier to understand than having
to use 'eS' for that.
> martijn@
>
> 1)
> Index: ber_get_string.3
> ===================================================================
> RCS file: /cvs/src/lib/libutil/ber_get_string.3,v
> retrieving revision 1.5
> diff -u -p -r1.5 ber_get_string.3
> --- ber_get_string.3 21 May 2019 12:30:07 -0000 1.5
> +++ ber_get_string.3 14 Aug 2019 12:30:17 -0000
> @@ -108,6 +108,12 @@ and
> the type of the element is not checked.
> For
> .Sq e ,
> +.Sq p ,
> +and
> +.Sq t
> +the pointer is not incremented to the next element.
> +For
> +.Sq e ,
> a pointer to the element is stored in the corresponding pointer variable.
> For
> .Sq S ,
>
>
> 2)
> Index: ber.c
> ===================================================================
> RCS file: /cvs/src/lib/libutil/ber.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 ber.c
> --- ber.c 14 Aug 2019 04:48:13 -0000 1.12
> +++ ber.c 14 Aug 2019 12:30:48 -0000
> @@ -711,7 +711,7 @@ ber_scanf_elements(struct ber_element *b
> e = va_arg(ap, struct ber_element **);
> *e = ber;
> ret++;
> - continue;
> + break;
> case 'E':
> i = va_arg(ap, long long *);
> if (ber_get_enumerated(ber, i) == -1)
>
--
:wq Claudio
