Comment 4 for bug 1452875

We talked about the general approach on IRC and Manoj will update the bug. We'll get the general approach agreed first. But while I'm looking at the patch, here are some review comments:

+enum _bool {false = 0, true = 1};
+typedef enum _bool Bool;

Please don't do this unless it's convention in the existing source. Use an int instead as this is standard C convention unless there's some reason to do otherwise.

+ char *platform = "HP ProLiant m800 Server Cartridge";
+ char *sys_file = "/proc/device-tree/model";

These should be declared const.

+ while (fp != NULL && fread(tmp, 1, sizeof(tmp), fp) != 0) {

Won't this go forever if there is some kind of unexpected error? It would be clearer to just return immediately on error - once for the return value of fopen and separately for the fread. I also don't think there's any need to run fread in a loop - if it fails the first time, then just treat the file as unreadable (and presumably fail the test negative). Or, if the /proc/device-tree/model file might be bigger than the 256 bytes you've allocated in tmp, then the approach needs to be different from just strstr().

+ "Balancing is ineffective on %s.\n", platform);

Please explain in the error message that this is the reason that irqbalance is exiting - otherwise it could be confused to a sysadmin wondering why irqbalance is exiting despite this particular message. Eg. "Balancing is ineffective on %s: exiting.\n"

+ if (disable_on_platform())

As this code will never be generic, can you name this disable_on_hp_m800 or similar please, so it's clearer to someone reading the code?

Finally, please add "Forwarded: not-needed" to the dep3 header together with an explanation of why forwarding is not needed in the description.