On Wed, Jul 24, 2013 at 11:55 PM, Dan Gora <dan.g...@gmail.com> wrote: > 0001-Re-work-of-ipmi_ek_display_fru_header_detail.patch > > This looks ok... It's not necessary to change > ipmi_ek_display_fru_header_detail to return int, especially since you > don't check the return code at all. Also the added storing of the > return code for fseek at the bottom, but not checking the value is > similarly specious, but harmless. > > 0002-Small-re-work-of-ipmi_ek_display_chassis_info_area.patch > > Same with this one.. Totally unnecessary to change the return value to > int since it's just displaying stuff and no one checks the return > code, storing the return value to fseek then not checking it doesn't > change the generated code at all, just adds extra verbiage to the > source, but is harmless. > > 0003-Small-changes-to-ipmi_ek_display_chassis_info_area.patch > > Same with this one.. >
Dan, I agree that some changes aren't as best as they could be, eg. checking some return values, and I admit there is still some work left. I hope code got simpler and compiler shut. Anyone is free to iterate on these changes and make code better. > So the changes to the patch that I submitted look completely > unnecessary, but harmless. Now, I don't know what you mean. I took your changes into account and, actually, they're there. I just wanted to iterate a bit more. > sorry for the original diff format. I have no idea how to drive CVS. No worries. I've applied your patch and then did % cvs diff -u ; to get unified output. Also, I hope all is in since I've split your patch into smaller chunks. In retrospect, I could have handled it differently and perhaps even better - like apply and commit your patch and then do the re-works. Well, next time. There are still those(two) scanf() patches left. I want to look at these closer, because it seems tricky to handle scanf() correctly. I mean, not just patch it up. I hope we'll be good to remove -Wno-unused-result after these two. > > I see you are using git as well.. Yes, especially if I want to change multiple things. I just check-in file/files, do the changes, export diffs. > Is there any chance we can get the > whole tree converted to git and get rid of this CVS stuff? I have no idea how to convert CVS to git(here at SF.net). But it's possible to have git, yes. Best regards, Z. > I'm sure > that most people nowadays are learning git, not CVS. > > > thanks > dan > > > > On Sun, Jul 21, 2013 at 1:44 PM, Zdenek Styblik > <zdenek.styb...@gmail.com> wrote: >> Hello all, >> >> attached are re-works of three functions in 'lib/ipmi_ekanalyzer.c'. >> These are based on patches Dan has uploaded/attached to ticket in >> question. >> Please, somebody do the review, wave the flag, so we can get a move on. >> >> Thanks, >> Z. > > > > -- > Dan Gora > Software Engineer > > Adax, Inc. > Av Dona Maria Alves, 1070 Casa 5 > Centro > Ubatuba, SP > CEP 11680-000 > Brasil > > Tel: +55 (12) 3833-1021 (Brazil and outside of US) > : +1 (510) 859-4801 (Inside of US) > : dan_gora (Skype) > > email: d...@adax.com ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk _______________________________________________ Ipmitool-devel mailing list Ipmitool-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
