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.
--
:wq Claudio
