On Fri, May 17, 2013 at 1:52 PM, Dmitry Bazhenov <dim...@pigeonpoint.com> wrote: > Hello, All! >
Hello Dmitry, my comments are below. > I have updated the patch of direct serial drivers. Now single and double > bridging is working. > Great. However, please fix the following lines: + rate = atol(p + 1); + rate = atol(p + 1); + rv = strtol(p + 4, &p, 16); + data[i++] = (unsigned char) strtol(str_hex, &pp, 16); 1] don't use atol() 2] since you have included 'ipmitool/helper.h' use appropriate str2*() function(s) instead of atol() and strtol() and be done with it. It seems to me like intf->session is leaking memory, resp. I don't see it being freed anywhere. There is a memory leak if '-D' is given more than once. I'm also wondering whether /dev/tty or /dev/ttyS couldn't be implied and whether extending 'struct ipmi_intf' is really necessary. But it's just a silly question, I guess. > Also, I found several cases where "tmp_pass" string acquired from the > "getpass()" call is incorrectly freed. The getpass() function returns > address to a static buffer and freeing that address results in abrupt > program termination. I removed these free() calls altogether. > Great, and I mean great. But let's do one thing at the time, ok? What I mean is it should, has to, be submitted separately. Also, I think your patch is incorrect. If this is issue *only* for getpass(), then fix should be something like: ~~~ #ifdef HAVE_GETPASSPHRASE free(tmp_pass); #endif ~~~ Thanks, Z. > Please, review. > > Regards, > Dmitry > > 26.04.2013 16:08, Dmitry Bazhenov пишет: > >> Hello, all, >> >> I would like to submit a patch which adds Serial Basic and Terminal mode >> interface drivers which utilize a direct serial connection in order to >> interact with IPMC. >> >> Both the patches implement single and double bridging. However, bridging >> is temporarily broken for PICMG systems due to the known bridging >> issues. I'm going to update the patch as soon as these issues are >> resolved. >> >> Regards, >> Dmitry > > > > ------------------------------------------------------------------------------ > AlienVault Unified Security Management (USM) platform delivers complete > security visibility with the essential security capabilities. Easily and > efficiently configure, manage, and operate all of your security controls > from a single console and one unified framework. Download a free trial. > http://p.sf.net/sfu/alienvault_d2d > _______________________________________________ > Ipmitool-devel mailing list > Ipmitool-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/ipmitool-devel > ------------------------------------------------------------------------------ AlienVault Unified Security Management (USM) platform delivers complete security visibility with the essential security capabilities. Easily and efficiently configure, manage, and operate all of your security controls from a single console and one unified framework. Download a free trial. http://p.sf.net/sfu/alienvault_d2d _______________________________________________ Ipmitool-devel mailing list Ipmitool-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
