Re: [LTP] [PATCH 09/13] testcases/kernel/mem: Move KSM bits to ksm tests

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> > new file mode 100644
> > index 000000000..ebce8197c
> > --- /dev/null
> > +++ b/testcases/kernel/mem/ksm/ksm_test.h
> ...
> > +static inline void ksm_child_memset(int child_num, int size, int total_unit,
> > +		 struct ksm_merge_data ksm_merge_data, char **memory)
> > +{
> > +	int i = 0, j;
> > +	int unit = size / total_unit;
> > +
> > +	tst_res(TINFO, "child %d continues...", child_num);
> > +
> > +	if (ksm_merge_data.mergeable_size == size * TST_MB) {
> This introduces new warnings, because the original code used MB which is long. 
> 
> #define MB                  (1UL<<20)
> 
> Now we use TST_MB, which is plain int.
> 
> In file included from ksm_common.h:16,
>                  from ksm04.c:42:
> ksm_test.h: In function ‘ksm_child_memset’:
> ksm_test.h:108:43: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Wsign-compare]
>   108 |         if (ksm_merge_data.mergeable_size == size * TST_MB) {
>       |                                           ^~
> ksm_test.h:119:45: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Wsign-compare]
>   119 |                 for (i = 0; (unsigned int)i < unit * TST_MB; i++)
>       |                                             ^
> ksm_test.h:126:43: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Wsign-compare]
>   126 |         if (ksm_merge_data.mergeable_size < size * TST_MB)
>       |                                           ^
> ksm_test.h: In function ‘create_ksm_child’:
> ksm_test.h:166:54: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Wsign-compare]
>   166 |                 if (ksm_merge_data[j].mergeable_size < size * TST_MB) {
>       |                                                      ^
> It would be nice to add cast before merge.

So the size is signed but it was implicitly casted to unsigned by the
TST_MB being unsigned. I guess that the best fix here is to make the
size and unit unsigned to begin with since these are numbers that should
not be negative at all.

> > +		tst_res(TINFO, "child %d allocates %d TST_MB filled with '%c'",
> 
> This should use MB (replace to whole file obviously should exclude strings):
> 
> 		tst_res(TINFO, "child %d allocates %d MB filled with '%c'",
> 
> > +			child_num, size, ksm_merge_data.data);
> > +
> > +	} else {
> > +		tst_res(TINFO, "child %d allocates %d TST_MB filled with '%c'"
> 
> And here as well.
> 		tst_res(TINFO, "child %d allocates %d MB filled with '%c'"
> > +				" except one page with 'e'",
> > +				child_num, size, ksm_merge_data.data);
> > +	}

Ah, sorry, that is a result from blindly doing sed s/...

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp