#449 ipmitool close console session for sol deactivate command

[Mamatha Inamdar]

In the new version of ipmitool 1.8.15 and above, sol deactivate command was not closing the session, This patch is to fix close console session When we issue sol deactivate command.

[Zdenek Styblik]

Hi,

I'm a bit confused, because of the following comment which is just right behind that if() block:

/*

 * Should recv_sol come back null, the incoming packet was not ours.
 * Just fall through, the keepalive logic will determine if
 * the BMC has dropped the session.
 */

[Mamatha Inamdar]

Yes above comments says keepalive logic will take care of BMC has droped the session or not. But what we have noticed is that after "ipmitool ... sol deactivate" BMC will close the active sol session but client side still have process running for sol deactivate for sometime..The time it will take to close on client side varies but it will be closed after sometime. In the meantime if user start the new session using "ipmitool ... sol activate" .... then sometime new session will be also closed/droped when sol deactivate process on client side close the socket/process.

[Zdenek Styblik]

Ok, more to the point. Your changes invalidate that comment, correct? Therefore that comment should be removed, resp. if true, then removal of that comment is mandatory.
Also, wouldn't it make more sense to rewrite function in question rather than just patching it over? Obviously, there is something wrong in there and giving it band aid will most likely lead to just more issues. Just my $0.02USD.

Also, please give a look to https://sourceforge.net/p/ipmitool/wiki/Home/ and https://sourceforge.net/p/ipmitool/wiki/Coding%20Standards/ how code should be formatted(mandatory) and patch generated(voluntary, but it will give you credit).

Thanks,
Z.

[Zdenek Styblik]

Just to make it clear. Musts:

1. patch must conform to coding standards
2. patch must remove comment which is being obsoleted by the patch
3. please, generate git patch in a way as described at wiki

Nice to have:

• perhaps give a closer look to the function in question since band aids can bring more issues

Thanks,
Z.

[Mamatha Inamdar]

I have attached the patch with the changes as suggested by you. your comments will be appreciated

[Zdenek Styblik]

Formatting was a bit off, but I've mended it. Thanks.