Re: [Xen-devel] [PATCH] numa.c: use spaces instead of tabs

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On mer, 2014-09-03 at 00:18 -0400, Elena Ufimtseva wrote:
> Signed-off-by: Elena Ufimtseva <ufimtseva@xxxxxxxxx>
> ---
>
I think the patch does what the subject says, i.e., converting tabs to
spaces, and does it right (I've only one question, see below).

However, does it make sense, while at it, to turn the file into a Xen's
code stile compliant one? Because, if yes, there are other things than
tabs-->spaces that needs fixing, I believe.

> --- a/xen/arch/x86/numa.c
> +++ b/xen/arch/x86/numa.c

> @@ -67,53 +67,53 @@ int srat_disabled(void)
>  static int __init populate_memnodemap(const struct node *nodes,
>                                        int numnodes, int shift, int *nodeids)
>  {
> -     unsigned long spdx, epdx;
> -     int i, res = -1;
> -
> -     memset(memnodemap, NUMA_NO_NODE, memnodemapsize * sizeof(*memnodemap));
> -     for (i = 0; i < numnodes; i++) {
> -             spdx = paddr_to_pdx(nodes[i].start);
> -             epdx = paddr_to_pdx(nodes[i].end - 1) + 1;
> -             if (spdx >= epdx)
> -                     continue;
> -             if ((epdx >> shift) >= memnodemapsize)
> -                     return 0;
> -             do {
> -                     if (memnodemap[spdx >> shift] != NUMA_NO_NODE)
> -                             return -1;
> -
> -                     if (!nodeids)
> -                             memnodemap[spdx >> shift] = i;
> -                     else
> -                             memnodemap[spdx >> shift] = nodeids[i];
> -
> -                     spdx += (1UL << shift);
> -             } while (spdx < epdx);
> -             res = 1;
> -     }
> -     return res;
> +    unsigned long spdx, epdx;
> +    int i, res = -1;
> +
> +    memset(memnodemap, NUMA_NO_NODE, memnodemapsize * sizeof(*memnodemap));
> +    for (i = 0; i < numnodes; i++) {
>
For example here. In Xen's style, this would become something like:

    for ( i = 0; i < numnodes; i++ )
    {

And the same throughout all the for-s and if-s in the file.

>  /*
> @@ -121,83 +121,83 @@ static int __init allocate_cachealigned_memnodemap(void)
>   * maximum possible shift.
>   */
>  static int __init extract_lsb_from_nodes(const struct node *nodes,
> -                                      int numnodes)
> +                     int numnodes)
>
Perhaps I am the one missing something here, but shouldn't this still be
aligned to the '(' above (through spaces, of course)?

Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel