Re: [Xen-devel] [PATCH] xenpaging:add a new array to speed up page-in in xenpaging

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

hongkaixing@xxxxxxxxxx writes ("[Xen-devel] [PATCH] xenpaging:add a new array 
to speed up page-in in xenpaging"):
> xenpaging:add a new array to speed up page-in in xenpaging

Oh, and a couple of style points.  You should keep to the style of the
code you're editing.  So:

> +    if (NULL == victims)

In the xenpaging code this is written like this:

       if (victims == NULL)

You should do the same, throughout.

> +    page_out_index = (victim_to_i_t *)calloc(paging->domain_info->max_pages, 
> sizeof(victim_to_i_t));

Do not cast the return value from malloc et al.

> +                i = page_out_index[req.gfn].index ;

The space before the semicolon is not the conventional style.

> +                    if( victims[i].gfn !=INVALID_MFN )

> -    free(victims);
> +    if ( NULL != victims )
> +    {
> +        free(victims);
> +    }

This is simply pointless.  free(NULL) is legal.

> +typedef struct victim_to_i {
> +    /* the index of victim array to read from */
> +    int index;
> +} victim_to_i_t;

Why wrap this up in a struct ?

Use of types with names ending in _t is reserved to the C
implementation (compiler and runtime).  I know xenpaging is full of
these but we shouldn't introduce any more.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel