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, jm...@redhat.com 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. > >>> Some small style comments highlighted below. >> >> I would like to have your eagle eyes...👀 Fixed them! >> >> Thanks, >> Jakob > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
