Em 30-04-2012 04:59, Borislav Petkov escreveu:

On Sun, Apr 29, 2012 at 11:16:53AM -0300, Mauro Carvalho Chehab wrote:

quoted

quoted

Hey, are you looking at compiled code or at source code? Because I'm
looking at source code, and it is a pretty safe bet the majority of the
people here do that too.

What I said is that, from source code POV, a code where the loop variables are
initialized just before the loop is easier to read it when the initialization
of those vars are on another part of the code.

That's basically why the "for" syntax starts with a var initialization clause.

The tot_dimms & friends are loop vars: their value is calculated within the loop.

At the object code, this won't bring any difference.

quoted

quoted

it, either by using registers for those vars or by moving the initialization
to the top of the function.

This function is too complex, so it is better to initialize those vars
just before the loops that are calculating those totals.

Simply initialize those variables at declaration time and that's it.
Initializing them before the loop doesn't make the function less complex
- splitting it and sanitizing it does.

Initializing loop-calculated vars just before the loop makes the code easier
to read, and may avoid issues that might happen during code lifecycle.

This is getting ridiculous:

With this I fully agree: you're nacking patches because it is not the way you
write your code, not because the code there is doing anything wrong.

If you point anything wrong on the way I wrote, then I'll fix. Otherwise, why
should I do a change that will obfuscate the code?

the variable declaration and initialization
are on the same screen as the loop (unless one uses a screen which can
only show less than 40ish lines).

So the argument about making the code easier to read is bogus.

This function is already cluttered with a lot of crap, and is very large
so adding more lines which can simply be stashed away at declaration
time is better readability.

Besides, every modern editor can jump to the declaration of a local
variable so that the user can see to what it is initialized to.

The editor used by te developer is not relevant. This is not a reason
to obfuscate the code.

quoted

+struct mem_ctl_info *new_edac_mc_alloc(unsigned edac_index,
+                                    unsigned n_layers,
+                                    struct edac_mc_layer *layers,
+                                    bool rev_order,
+                                    unsigned sz_pvt)
 {
      void *ptr = NULL;
      struct mem_ctl_info *mci;
-     struct csrow_info *csi, *csrow;
+     struct edac_mc_layer *layer;
+     struct csrow_info *csi, *csr;
      struct rank_info *chi, *chp, *chan;
      struct dimm_info *dimm;
+     u32 *ce_per_layer[EDAC_MAX_LAYERS], *ue_per_layer[EDAC_MAX_LAYERS];
      void *pvt;
-     unsigned size;
-     int row, chn;
+     unsigned size, tot_dimms, count, pos[EDAC_MAX_LAYERS];
+     unsigned tot_csrows, tot_channels, tot_errcount = 0;
+     int i, j;
      int err;
+     int row, chn;
+     bool per_rank = false;
+
+     BUG_ON(n_layers > EDAC_MAX_LAYERS || n_layers == 0);
+     /*
+      * Calculate the total amount of dimms and csrows/cschannels while
+      * in the old API emulation mode
+      */
+     tot_dimms = 1;
+     tot_channels = 1;
+     tot_csrows = 1;
+     for (i = 0; i < n_layers; i++) {
+             tot_dimms *= layers[i].size;
+             if (layers[i].is_virt_csrow)
+                     tot_csrows *= layers[i].size;
+             else
+                     tot_channels *= layers[i].size;
+
+             if (layers[i].type == EDAC_MC_LAYER_CHIP_SELECT)
+                     per_rank = true;

Regards,
Mauro