On 3/19/24 13:21, Ilya Maximets wrote: > On 3/19/24 13:17, Eelco Chaudron wrote: >> >> >> On 19 Mar 2024, at 13:11, Jakob Meng wrote: >> >>> Hi! >>> >>> On 15.03.24 11:19, Eelco Chaudron wrote: >>>> On 18 Jan 2024, at 16:26, [email protected] wrote: >>>> >>>>> ... >>>> Thank for the patch! What a beast to go trough ;) >>> >>> Thank you for doing it anyway ☺️ >>> >>>> I believe the current approach is acceptable. However, we could also >>>> incorporate union callbacks: if registration only supports text, we would >>>> use callback A, and if multiple formats exist, we could employ the new >>>> style >>>> callback. This would mitigate the need for a significant overhaul. Just a >>>> suggestion, as I'm ok with the current approach. :) >>> >>> This trades many-initial-changes-across-a-lot-of-files for a >>> more-complex-registration-and-callback-logic. >>> >>> We could also make struct unixctl_conn visible (move it from lib/unixctl.c >>> to lib/unixctl.h) and let callees read the output format from its 'fmt' >>> member. Then we would not have to change the callback signature at all. >> >> Or have a utility function to return the format, which keeps the structure >> invisible. > > +1, just add some access methods. > >> >>> I do not have a strong opinion here. What do you, Ilya and the others think? >> >> I’ll wait for others to chime in... > > I don't think changing all the callbacks is a good thing to do. > i.e. this patch should not exist or be very small.
* or it should be very small. > >> >>>> Some small style comments highlighted below. >>> >>> I would like to have your eagle eyes...👀 Fixed them! >>> >>> Thanks, >>> Jakob >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
