| Message ID | 1456411743-17741-8-git-send-email-george.dunlap@eu.citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
George Dunlap writes ("[PATCH 7/8] tools/xenalyze: Fix multiple instances of *HYPERCALL_MAX"):
> We HYPERCALL_MAX defined as the maximum enumerated hypercall, and we
^ missing word `have' ?
> have PV_HYPERCALL_MAX defined as some other number (presumably based
> on experience with actual hypercalls). Both are used to size arrays
> (hypercall_name[] and pv_data.hypercall_count[], respectively).
>
> Rename PV_HYPERCALL_MAX to HYPERCALL_MAX, and use HYPERCALL_MAX to
> size (and iterate over) all arrays.
...
> diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c
> index 3e26a4c..4ae50b8 100644
> --- a/tools/xentrace/xenalyze.c
> +++ b/tools/xentrace/xenalyze.c
> @@ -1068,9 +1068,10 @@ enum {
> HYPERCALL_sysctl,
> HYPERCALL_domctl,
> HYPERCALL_kexec_op,
> - HYPERCALL_MAX
> };
>
> +#define HYPERCALL_MAX 38
> +
> char *hypercall_name[HYPERCALL_MAX] = {
> [HYPERCALL_set_trap_table]="set_trap_table",
> [HYPERCALL_mmu_update]="mmu_update",
> @@ -1509,13 +1510,12 @@ char *pv_name[PV_MAX] = {
> [PV_HYPERCALL_SUBCALL]="hypercall (subcall)",
Does this produce a build error if HYPERCALL_MAX is too small ?
Ian.
On 26/02/16 12:33, Ian Jackson wrote: > George Dunlap writes ("[PATCH 7/8] tools/xenalyze: Fix multiple instances of *HYPERCALL_MAX"): >> We HYPERCALL_MAX defined as the maximum enumerated hypercall, and we > ^ missing word `have' ? > >> have PV_HYPERCALL_MAX defined as some other number (presumably based >> on experience with actual hypercalls). Both are used to size arrays >> (hypercall_name[] and pv_data.hypercall_count[], respectively). >> >> Rename PV_HYPERCALL_MAX to HYPERCALL_MAX, and use HYPERCALL_MAX to >> size (and iterate over) all arrays. > ... >> diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c >> index 3e26a4c..4ae50b8 100644 >> --- a/tools/xentrace/xenalyze.c >> +++ b/tools/xentrace/xenalyze.c >> @@ -1068,9 +1068,10 @@ enum { >> HYPERCALL_sysctl, >> HYPERCALL_domctl, >> HYPERCALL_kexec_op, >> - HYPERCALL_MAX >> }; >> >> +#define HYPERCALL_MAX 38 >> + >> char *hypercall_name[HYPERCALL_MAX] = { >> [HYPERCALL_set_trap_table]="set_trap_table", >> [HYPERCALL_mmu_update]="mmu_update", >> @@ -1509,13 +1510,12 @@ char *pv_name[PV_MAX] = { >> [PV_HYPERCALL_SUBCALL]="hypercall (subcall)", > > Does this produce a build error if HYPERCALL_MAX is too small ? You mean, if it's smaller than at least one of the indexes in the array initialization immediately following? Yes. (I just tested it to be sure.) -G
George Dunlap writes ("Re: [PATCH 7/8] tools/xenalyze: Fix multiple instances of *HYPERCALL_MAX"):
> On 26/02/16 12:33, Ian Jackson wrote:
> > Does this produce a build error if HYPERCALL_MAX is too small ?
>
> You mean, if it's smaller than at least one of the indexes in the array
> initialization immediately following? Yes. (I just tested it to be sure.)
Right. Jolly good.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c index 3e26a4c..4ae50b8 100644 --- a/tools/xentrace/xenalyze.c +++ b/tools/xentrace/xenalyze.c @@ -1068,9 +1068,10 @@ enum { HYPERCALL_sysctl, HYPERCALL_domctl, HYPERCALL_kexec_op, - HYPERCALL_MAX }; +#define HYPERCALL_MAX 38 + char *hypercall_name[HYPERCALL_MAX] = { [HYPERCALL_set_trap_table]="set_trap_table", [HYPERCALL_mmu_update]="mmu_update", @@ -1509,13 +1510,12 @@ char *pv_name[PV_MAX] = { [PV_HYPERCALL_SUBCALL]="hypercall (subcall)", }; -#define PV_HYPERCALL_MAX 56 #define PV_TRAP_MAX 20 struct pv_data { unsigned summary_info:1; int count[PV_MAX]; - int hypercall_count[PV_HYPERCALL_MAX]; + int hypercall_count[HYPERCALL_MAX]; int trap_count[PV_TRAP_MAX]; }; @@ -6405,7 +6405,7 @@ void pv_hypercall_process(struct record_info *ri, struct pv_data *pv) { } if(opt.summary_info) { - if(eax < PV_HYPERCALL_MAX) + if(eax < HYPERCALL_MAX) pv->hypercall_count[eax]++; } @@ -6566,10 +6566,10 @@ void pv_summary(struct pv_data *pv) { switch(i) { case PV_HYPERCALL: case PV_HYPERCALL_V2: - for(j=0; j<PV_HYPERCALL_MAX; j++) { + for(j=0; j<HYPERCALL_MAX; j++) { if(pv->hypercall_count[j]) printf(" %-29s[%2d]: %6d\n", - hypercall_name[j], + hypercall_name[j]?:"(unknown)", j, pv->hypercall_count[j]); } @@ -6677,7 +6677,7 @@ void pv_hypercall_v2_process(struct record_info *ri, struct pv_data *pv, int op = pv_hypercall_op(ri); if(opt.summary_info) { - if(op < PV_HYPERCALL_MAX) + if(op < HYPERCALL_MAX) pv->hypercall_count[op]++; }
We HYPERCALL_MAX defined as the maximum enumerated hypercall, and we have PV_HYPERCALL_MAX defined as some other number (presumably based on experience with actual hypercalls). Both are used to size arrays (hypercall_name[] and pv_data.hypercall_count[], respectively). Rename PV_HYPERCALL_MAX to HYPERCALL_MAX, and use HYPERCALL_MAX to size (and iterate over) all arrays. While we're at it, print "(unknown)" for values not present in the hypercall_name[] array. CID 1306868 Signed-off-by: George Dunlap <george.dunlap@citrix.com> --- CC: Ian Jackson <ian.jackson@citrix.com> CC: Wei Liu <wei.liu2@citrix.com> --- tools/xentrace/xenalyze.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)