On Thu, Sep 17, 2020 at 12:49 AM Dumitru Ceara <[email protected]> wrote: > > On 9/16/20 10:05 PM, Han Zhou wrote: > >> > Same as above. In addition, I prefer fewer functions because it is > >> > easier (slightly) to tell from the prototypes that those are the only > >> > operations possible for this data structure. In this case, it tells that > >> > this is the only interface that allocates memory for the resource > >> > reference table. Of course with extra functions you can still tell that > >> > by checking the code and realize that there is only one place the > >> > _create() functions are called, but to me that is just more noise > >> > (slightly). I don't think every data structure needs all the > >> > constructors and destructors in C. The code should evolve naturally when > >> > such functions are necessary. But if there is a coding style kind of > >> > agreement that makes it a convention then I would just follow. Again, I > >> > don't have a strong opinion in this case. > >> > > >> > >> I tend to agree in general but my concern in this specific case is that > >> it's very easy to make a typo and write "free(lfrn);" instead of > >> "free(lrln);". > >> > > True. But that's more of a variable naming problem. Some may argue that > > it is easy to call the wrong function if the function names are similar :) > > Well, even if the function names are similar, they would be operating on > different data types so the compiler would complain if we'd try to call > the wrong function. > > While free(void *) compiles with any pointer arg :)
I meant some like: ref_lflow_node_destroy(rlfn); v.s. lflow_ref_node_destroy(lfrn); Or even worse: rl_node_destroy(rlfn); v.s. lr_node_destroy(lfrn); Both would compile, although functions are used. On the other hand, it is less likely to make a mistake even with just free: free(resource_node); v.s. free(logical_flow_node); So I'd say the real ones to blame in such cases are probably the naming + careless programmer and reviewer :) > > In any case, I'll give this more thought coming weeks and I'll try to > see if I can come up with a concrete patch we can discuss on. Appreciate it! > > Thanks, > Dumitru > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
