On 8/14/19 4:48 PM, Claudio Jeker wrote:
> On Wed, Aug 14, 2019 at 04:37:48PM +0200, Martijn van Duren wrote:
>> On 8/14/19 3:35 PM, Claudio Jeker wrote:
>>> 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.
>>
>> So which one do you prefer? If we document 1 it's somewhat final.
>
> Not necessarly final. Anyway, I would prefer 2.
> Still part of the diff should still go in since t and p still don't
> progress.
>
>> Also: Yes it's easier and more intuitive to do "e" instead of "eS",
>> but if you need both the ber_element and it's value it becomes
>> impossible to do so in a single run, resulting (in some cases) in
>> more code and more runtime. So I'm not convinced either one is
>> better.
>
> If you use 'e' it returns a ber_element this element includes the value
> (you just need to use one of the ber_get* functions on it). If you fetch
> the value then use 't' to get the class and type. Not sure if there is
> anything else that that would be needed. This is the reason why I think
> doing 2 is making the API better.
>
So that would be the following diff:
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 14:49:57 -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)
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 14:49:57 -0000
@@ -107,6 +107,11 @@ and
.Sq t ,
the type of the element is not checked.
For
+.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
