Hi Hemantha, Thanks for formatting the code to 80 lines. Here are my review comments. a. ir_xml_scan_temperature function help at the start of the function says Return Value None, but it does return an int. b. Call to process temperature data goes here comment could be removed from the ir_xml_scan_temperature function c. The return value ret gets assigned, but the function always returns RIBCL_SUCCESS! d. ir_xml_record_temperaturedata function variables could be initialized e. ir_xml_record_temperaturedata temperaturelocation has two location strings in the help f. ir_xml_record_temperaturedata the passed in values need to be validated (at least make sure ir_handler != NULL) g. Even if ir_handler != NULL, accessing ir_handler->DiscoveryData.chassis_sensors[temperatureindex+3] could core dump. Should this be tsdata instead of chassis_sensors? tsdata is the field that is never populated but chassis_sensors is. h. sensordat->discription. Is this even needed? i. The discover file also will change if tsdata is the right one to use.
Mohan On Tue, 2011-02-08 at 09:25 +0000, Beecherla, Hemantha wrote: > Hi Bryan, > I have formatted the lines in the new code to <=80 columns, according to > the project guidelines. > Please find the attached patch for review and let me know if any comments. > > Thanks& Regards, > Hemantha Reddy > > ________________________________________ > From: Sutula, Bryan (Open Source Program Office) > Sent: Monday, February 07, 2011 5:48 PM > To: [email protected] > Subject: Re: [Openhpi-devel] Patch for bug Source Forge ID: 3109793(Populate > Temperature Sensors) to review > > On Mon, 2011-02-07 at 06:42 +0000, Beecherla, Hemantha wrote: > > I have fixed the bug Source Forge ID: 3109793(Populate Temperature > > Sensors) and attached the patch(3109793.patch) file. > > PFA patch and logs for review, and let me know the comments if any. > > Would you please try to format the new source code to be <= 80 columns > according to the project guidelines listed here: > > http://openhpi.org/Developers > > and the http://openhpi.org/LinuxKernelCodingStyle linked from that page? > > I know that there are places where the current code does not meet that > guideline, but it's helpful for maintenance if we try to correct these > when we can, and avoid adding new code with long lines. > > Thanks, > Bryan > > > ------------------------------------------------------------------------------ > The modern datacenter depends on network connectivity to access resources > and provide services. The best practices for maximizing a physical server's > connectivity to a physical network are well understood - see how these > rules translate into the virtual world? > http://p.sf.net/sfu/oracle-sfdevnlfb > _______________________________________________ > Openhpi-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/openhpi-devel > ------------------------------------------------------------------------------ > The ultimate all-in-one performance toolkit: Intel(R) Parallel Studio XE: > Pinpoint memory and threading errors before they happen. Find and fix more > than 250 security defects in the development cycle. Locate bottlenecks in > serial and parallel code that limit performance. > http://p.sf.net/sfu/intel-dev2devfeb > _______________________________________________ > Openhpi-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/openhpi-devel ------------------------------------------------------------------------------ The ultimate all-in-one performance toolkit: Intel(R) Parallel Studio XE: Pinpoint memory and threading errors before they happen. Find and fix more than 250 security defects in the development cycle. Locate bottlenecks in serial and parallel code that limit performance. http://p.sf.net/sfu/intel-dev2devfeb _______________________________________________ Openhpi-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openhpi-devel
