Hello, I would ACK it, partially due to backwards compatibility. Rework proposal below. Should not it be checking only <0, 192) and leave the rest to the OEM code?
Although I expect this place to be changing soon. The rest is more of a know this part a bit more then really a review. I see the parameters are identified by ipmi_lanp.h enum. This place is being modified also by one oem patch pending on my list. https://bugzilla.redhat.com/attachment.cgi?id=705662 hunk @@ -79,10 +79,17 @@ These are wire protocol id numbers, right? Spec table 23-2 parameter selector. And the 192 and above OEM reserved, Table 23-4. My question really is about the OEM parameters 192 and above with conflicting OEM overlap. Since the code does not seem to be prepared to handle that unless I am overlooking something. If so then the 192+ commands are split according to spec: The OEM is identified according to the Manufacturer ID field returned by the Get Device ID command. ----- Original Message ----- From: "Zdenek Styblik" <zdenek.styb...@gmail.com> To: "ipmitool-devel" <ipmitool-devel@lists.sourceforge.net> Cc: "Jim Mank" <jm...@hp.com>, "Ales Ledvinka" <aledv...@redhat.com> Sent: Friday, March 8, 2013 9:45:05 PM Subject: Code Review - ID: 3600926 - 'lib/ipmi_lanp.c' - check LAN param Hello all, attached is a diff of proposed change to 'lib/ipmi_lanp.c'. This bug has been reported by Ales Ledvinka a the problem is this ``if (p == NULL)'' condition is never met, because struct is on heap, if I understood and writing it correctly. Comments? Ideas? I've tested it on small snippet code and it seems to work just fine. It's possible such change will be required at more places in 'lib/ipmi_lanp.c'. Thanks, Z. ------------------------------------------------------------------------------ Symantec Endpoint Protection 12 positioned as A LEADER in The Forrester Wave(TM): Endpoint Security, Q1 2013 and "remains a good choice" in the endpoint security space. For insight on selecting the right partner to tackle endpoint security challenges, access the full report. http://p.sf.net/sfu/symantec-dev2dev _______________________________________________ Ipmitool-devel mailing list Ipmitool-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
