On Thu, Jun 15, 2017 at 02:50:17PM +0300, Matan Barak wrote: > Following object oriented approach, I can think of the following > trivial changes: > type->class > action->method (actually, these are static methods, but nm) I don't think they are static methods.. anything that takes an object id or a fd num is a normal method acting on that object, anything that returns an object id/fd num is a constructor, anything that destroys an object id is a destructor. > The "groups" concept is a bit harder to grasp in OO. In our case, an > entity is actually divided into a few "groups" and its actual > content This is similar to standard OO concept of mixins/multiple inheritance, but you are applying the idea to function parameter lists, not classes. > In contrast of the OO multiple inheritance case, we have a specific > meaning to the order of these groups - the handler actually uses > them. That distinction seems conceptually unnecessary.. The access of the arguments should not depend on where the argument is defined, only on the argument ID, which is back to what I said before, why do we need to even have a word for namepace? static inline void *uverbs_get_attr(const uvbers_attr_bundle *args, uint32_t attr_id) { if (((attr_id & UVERBS_ID_NS_MASK) >> UVERBS_ID_NS_SHIFT) == 0) return args.common_attrs[attr_id & XX]; if (((attr_id & UVERBS_ID_NS_MASK) >> UVERBS_ID_NS_SHIFT) == 1) return args.driver_attrs[attr_id & YY]; return NULL; } void some_method(const uvbers_attr_bundle *args) { uverbs_get_attr(args, ATTR_ID_COMMON_A); uverbs_get_attr(args, ATTR_ID_MLX5_B); } > struct uverbs_attr_spec { > enum uverbs_attr_type type; > <...> > }; > > struct uverbs_attr_spec_ns { > struct uverbs_attr_spec *attrs; > <...> > }; I don't think we need _ns versions of any of these at all. Stop treating ns as special, there is only attribute IDs, and the only time the specialness of the ID layout comes into play is when you hash the ID for quick lookup, or 'hash' the ID for attribute storage (eg uverbs_get_attr) *NOTHING* else should care if an ID is within the driver, common or other reserved space. > >> I used the term "group" to emphasize that these aren't just a random > >> collection of params in a lit. > >> Grouping this entities together has a specific meaning. > > > > But *what* is that meaning? That's what I'd like the name to > > convey. I thought the grouping was because the attributes were a > > set of parameters being passed into a single > > function/operation/action/method. > > Entities were grouped together because they belong to the same > namespace. If we take the attributes for an example, once we execute > the method's handler, we get one group of attributes for the common > namespace and another one for the driver namespace. By using > namespaces, we could introduce new arguments for either common and > driver specific without stepping on other attributes in different > namespaces. When reaching the handler, each argument has a unique > meaning. Please see the uverbs_attr_array example above. Again, don't see why we should care. All of this micro-optimizing should be hidden from the method author. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html