Dear David, Can you please reply?
Thanks, Sandeep. On Thu, Mar 13, 2014 at 11:47 PM, Sandeep K Chaudhary < [email protected]> wrote: > Hi David, > > What about the following changes? > > 1071a1072,1079 > > > > bool check_conn_version(struct relay_connection *conn, uint32_t major, > uint32_t minor) { > > if((conn->major == major) && (conn->minor == minor)) > > return true; > > > > return false; > > } > > > 1097,1103c1105 > < switch (conn->minor) { > < case 1: > < case 2: > < case 3: > < break; > < case 4: /* LTTng sessiond 2.4 */ > < default: > --- > > if(check_conn_version(conn, conn->major, 4)) > 1105d1106 > < } > 1185,1193c1186,1191 > < switch (conn->minor) { > < case 1: /* LTTng sessiond 2.1 */ > < ret = cmd_recv_stream_2_1(conn, stream); > < break; > < case 2: /* LTTng sessiond 2.2 */ > < default: > < ret = cmd_recv_stream_2_2(conn, stream); > < break; > < } > --- > > if(check_conn_version(conn, conn->major, 1)) > > ret = cmd_recv_stream_2_1(conn, stream); > > > > else if(check_conn_version(conn, conn->major, 2)) > > ret = cmd_recv_stream_2_2(conn, stream); > > > > On Thu, Mar 13, 2014 at 7:44 AM, David Goulet <[email protected]>wrote: > >> On 12 Mar (22:59:20), Sandeep K Chaudhary wrote: >> > Thanks for pointing it to me, David ! >> > >> > However I see the following in relay_add_stream in main.c >> > >> > switch (conn->minor) { >> > case 1: /* LTTng sessiond 2.1 */ >> > ret = cmd_recv_stream_2_1(conn, stream); >> > break; >> > case 2: /* LTTng sessiond 2.2 */ >> > default: >> > ret = cmd_recv_stream_2_2(conn, stream); >> > break; >> > } >> > >> > This means that it's calling either 'cmd_recv_stream_2_1' or >> > 'cmd_recv_stream_2_2' depending on the minor version number. Is this >> not we >> > want? Though I don't understand why it should be calling >> > 'cmd_recv_stream_2_2' for minor versions other than 1. Is this correct >> > behavior? >> >> Yes so for now only the minor version matter since a major number other >> than 2 can't be negotiated during the send_version step (the first thing >> the relayd receives for a connection). >> > > In the proposed changes, we simply call the version checker function with > the same major version as in conn. In future, the calls can be made with > desired major and minor version numbers. > > >> >> At some point in time, we would need to check the major number to here >> so calling cmd_recv_stream_2_2(...) would make more sense. Maybe it's >> something you want to do here before anything, adding a layer that >> checks major + minor and calls the right one? Something that could be >> generic enough so we don't have those ugly switch cases in each >> command... ? >> > > Yes, I understand. And this would make the checking generic and better > than switch statements. Please see the proposed changes and let me know > what you think about it. Though, I have deliberately not taken care of the > 'default' case in earlier switch statements. I am not convinced > why cmd_create_session_2_4 (in relay_create_session) > and cmd_recv_stream_2_2 ( in relay_add_stream) should be called in default > cases. > > Thanks and regards, > Sandeep. > -- Thanks and regards, Sandeep K Chaudhary.
_______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
