[lttng-dev] Working on some bugs

[Sandeep K Chaudhary]

Dear David,

Can you please reply?

Thanks,
Sandeep.

On Thu, Mar 13, 2014 at 11:47 PM, Sandeep K Chaudhary <
babbusandy2006 at gmail.com> 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 <dgoulet at efficios.com>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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lttng.org/pipermail/lttng-dev/attachments/20140316/de9430cf/attachment.html>