On Tue, 1 Aug 2023 10:00:01 +0800 (GMT+08:00) Lin Ma wrote: > > > However, this is tedious and just like Leon said: add another layer of > > > cabal knowledge. The better solution should leverage the nla_policy and > > > discard nlattr whose length is invalid when doing parsing. That is, we > > > should defined a nested_policy for the X above like > > > > Hard no. Putting array index into attr type is an advanced case and the > > parsing code has to be able to deal with low level netlink details. > > Well, I just known that the type field for those attributes is used as array > index. > Hence, for this advanced case, could we define another NLA type, maybe > NLA_NESTED_IDXARRAY enum? That may be much clearer against modifying existing > code. What if the value is of a complex type (nest)? For 10th time, if someone does "interesting things" they must know what they're doing. > > Higher level API should remove the nla_for_each_nested() completely > > which is rather hard to achieve here. > > By investigating the code uses nla_for_each_nested macro. There are basically > two scenarios: > > 1. manually parse nested attributes whose type is not cared (the advance case > use type as index here). > 2. manually parse nested attributes for *one* specific type. Such code do > nla_type check. > > From the API side, to completely remove nla_for_each_nested and avoid the > manual parsing. I think we can choose two solutions. > > Solution-1: add a parsing helper that receives a function pointer as an > argument, it will call this pointer after carefully verify the > type and length of an attribute. > > Solution-2: add a parsing helper that traverses this nested twice, the first > time to do counting size for allocating heap buffer (or stack > buffer from the caller if the max count is known). The second > time to fill this buffer with attribute pointers. > > Which one is preferred? Please enlighten me about this and I can try to propose > a fix. (I personally like the solution-2 as it works like the existing parsers > like nla_parse) Your initial fix was perfectly fine. We do need a solution for a normal multi-attr parse, but that's not the case here.
