On Mon, May 20, 2013 at 6:31 AM, Dmitry Bazhenov <dim...@pigeonpoint.com> wrote: [...] >> 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); > > > I have changed atol() calls to str2uint(). > Unfortunately, ipmitool/helper.h does not provide appropriate API to replace > strtol(). In the code strtol() is called with radix=16 that means the input > is read in hexadecimal form only. I think it is best to leave the strtol() > calls there as they are. >
I see. Well, this sucks. Two solutions come to my mind. The first is to extend str2*() calls which, I admit, is going to be a bit of work. The second is to check 'errno', endptr and '#include <limits.h>'. In other words, do what str2*() calls do. [...] >> >> 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. > > > I couldn't find a better solution to provide a device string to the drivers. > The implying /dev/tty(s, USB) strings in the command-line is possible. I'll > think through how to make it without affecting those users who get used to > specify the full device names. > No, you're right. Serial dev can have <whatever> name. I guess it's fine as it's. [...] > > These two functions both return a pointer to the static data. Hence, the > #ifdef guard is unnecessary. > Hmm :-S Ok. To be honest, I'm disappointed, because this means possible memory leak which "can't" be fixed. Thanks, Z. > I'll provide the updated patch for serial drivers and separate patch for the > bug soon. > > No more comments below. > > Regards, > Dmitry > > >> >> 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
