#276 HPM.1 upgrade combined patch

[Dmitry Bazhenov]

This patch does the following:

• Fixes percentage visualization for short upgrade images
• Resolves ambiguity for action record checksum calculation
• Skips the activation stage when all components are skipped
• Partially fixes formatting
• Adds several code optimizations

[Zdenek Styblik]

Ticket moved from /p/ipmitool/patches/73/

[Zdenek Styblik]

sigh SF.net got borked really bad since they've moved to the new design. I've already spent like 30 minutes trying to apply formatting to the text and it still looks like a crap and actually is borked.

---

    19 out of 80 hunks FAILED -- saving rejects to file lib/ipmi_hpmfwupg.c.rej

Please, can you resync the patch? :\ And I have some questions too.

This doesn't look right, does it?

• If I remember correctly, C/gcc has these functions.
• even if it doesn't, you should do something like #ifndef MIN
• something like this belongs to header file
• ./include/ipmitool/ipmi_delloem.h has this function already
• sadly, delloem has it in the very same manner - explicit definition

  @@ -211,6 +223,9 @@
  #define HPMFWUPG_IS_RETRYABLE(error) FALSE
  #endif
  
  +#define MIN(a, b) ((a) < (b) ? (a) : (b))
  +#define MAX(a, b) ((a) > (b) ? (a) : (b))
  +
   /*
  
    *  HPM FIRMWARE UPGRADE GENERAL DEFINITIONS
    */
  

Why have you removed these definitions?

    :::C
    @@ -1001,9 +1016,7 @@
     #define VERSIONCHECK_MODE             0x01
     #define VIEW_MODE                     0x02
     #define DEBUG_MODE                    0x04
    -#define FORCE_MODE_ALL                0x08
    -#define FORCE_MODE_COMPONENT          0x10
    -#define FORCE_MODE                    (FORCE_MODE_ALL|FORCE_MODE_COMPONENT)
    +#define FORCE_MODE                    0x08

     typedef struct _VERSIONINFO
     {

You've missed ``uploadCmd.req = NULL;'' here.

    :::C
    @@ -2468,15 +2526,23 @@
    [...]
    +

    +      /* free buffer */
    +      free(uploadCmd.req);
        }
    [...]

What exactly is the point of this change?

    :::C
    @@ -1102,7 +1117,7 @@
     *****************************************************************************/
     int HpmGetUserInput(char *str)
     {

    -    char userInput[2];
    +    unsigned char userInput[2];
         printf("%s", str);
         scanf("%s",userInput);
         if (toupper(userInput[0]) == 'Y')

[Zdenek Styblik]

No comments on formatting, because hpmfwum needs clean up. So I'll let you off there this time :-P

[Dmitry Bazhenov]

Hello, Zdenek. Thanks for review. I will bring the patch up to date and answer to your comments as I get rid of my current tasks. It may take some time though.

[Zdenek Styblik]

Ok Dmitry, no worries.

[Dmitry Bazhenov]

Hello, Zdenek,

I have revised the patch and addressed your comments in the code.
The updated patch is attached. It solves many bug-fixes and adds some new funcionality.
The list of changes is placed in the changelog of the file and is a part of the patch.
Unfortunately, I'm not able to actively address comments any more, so, if they happen, it may take considerable time before I address them.

Regarding the comment about FORCE_MODE stuff. My patch slightly changes the user interface of the hpm upgrade command. Now, several "component x" parameters are allowed to select several components to upload in the multi-component image. If no "component x" is specified, all components are selected. The force parameter allows to bypass the component version check and forcefully upgrqade the components.

Regards,
Dmitry

[Zdenek Styblik]

Committed into CVS.