On Mon, Mar 11, 2013 at 6:55 PM, Ales Ledvinka <aledv...@redhat.com> wrote: > Oh and noticed the tracker comment about the bigger issue.
Yes, get_lan_param_select() may return NULL, yet it's not being checked what the returned value is, resp. whether it's NULL or isn't. NULL reference in 3 ... 2 ... 1 > If you are testing > with current cvs code I guarantee you will run into a plenty of double-free > bugs > after the leaks fixes. So if it was related to that... > Interesting, but I advise you to put piece of broken code or, even better, SF.net ticket where your mouth is. Because it might seem like it's just a dust coming out of your mouth. Best regards, Zdenek > ----- Original Message ----- > From: "Ales Ledvinka" <aledv...@redhat.com> > To: "Zdenek Styblik" <zdenek.styb...@gmail.com> > Cc: "ipmitool-devel" <ipmitool-devel@lists.sourceforge.net> > Sent: Monday, March 11, 2013 6:32:09 PM > Subject: Re: [Ipmitool-devel] Code Review - ID: 3600926 - 'lib/ipmi_lanp.c' > - check LAN param > > 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 ------------------------------------------------------------------------------ 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
