[LTP] [PATCH v1] float: convert to new LTP API

[Hongchen Zhang]

On 2022/7/26 下午7:55, Petr Vorel wrote:
> Hi Hongchen,
> 
> thank you for your effort, but much more needs to be done.
> 
> Code in whole project (all sources in testcases/misc/math/float) is very old and
> IMHO weird. I still wonder why testing float / math functions requires creating
> files, using pthread (in thread_code.c), ... It'd be worth to have look whether
> current approach is really useful before spending time to rewrite it.
> 
> Some notes to your rewrite. main.c and thread_code.c should be turned into
> header file (e.g. float.h) with functions being static inline. Because
> including C files is no-go.
> 
>>   testcases/misc/math/float/main.c | 446 +++++++++------------------------------
> ...
>> -const int nb_func = NB_FUNC;
>> +static char *Dopt, *lopt, *nopt, *vopt;
>> +static struct tst_option opt[] = {
>> +	{"D:", &Dopt,   "DATA directory's absolute path"},
> IMHO this should not be needed, everything should be in test temporary
> directory, there is no need to put it elsewhere.
>> +	{"l:", &lopt, "set number of loops per function"},
>> +	{"n:", &nopt, "set number of threads per function"},
>> +	{"v", &vopt, "debug level"},
> I'd get rid of the debugging (important things should be always printed).
> 
>> +	{}
>> +};
> 
>>   int generate(char *datadir, char *bin_path)
>>   {
>>   	char *cmdline;
>>   	char *fmt = "cd %s; %s/%s %s";
> 
>> -	cmdline = malloc(2 * strlen(bin_path) + strlen(datadir) + strlen(GENERATOR) + strlen(fmt));
>> +	cmdline = malloc(2 * strlen(bin_path) + strlen(datadir) +
>> +				strlen(GENERATOR) + strlen(fmt));
>>   	if (cmdline == NULL)
>>   		return (1);
> There is SAFE_MALLOC(), no need to check for NULL.
> 
>>   	sprintf(cmdline, fmt, datadir, bin_path, GENERATOR, bin_path);
>> @@ -93,345 +55,137 @@ int generate(char *datadir, char *bin_path)
>>   	return (0);
> 
> Also code style suggests it's very old. brackets around integer in return is
> quite strange (i.e. "return (0);").
>>   }
> 
> 
>>   	ltproot = getenv("LTPROOT");
>>   	if (ltproot == NULL || strlen(ltproot) == 0) {
>> -		tst_brkm(TBROK, NULL,
>> +		tst_brk(TBROK,
>>   			 "You must set $LTPROOT before executing this test");
> 
> generate() function which runs binary should be replaced with tst_cmd().
> IMHO we don't need to check for $LTPROOT, because we expect PATH to be set
> correctly.
> 
>>   	}
>>   	bin_path = malloc(strlen(ltproot) + 16);
>>   	if (bin_path == NULL) {
> SAFE_MALLOC() (in many places)
>> -		tst_brkm(TBROK | TERRNO, NULL, "malloc failed");
>> +		tst_brk(TBROK, "malloc failed");
>>   	}
> ...
> 
>> +void run(unsigned int n)
>> +{
>> +	void *exit_value;
>> +	pthread_attr_t newattr;
>> +	size_t stacksize = 2093056;
> I'm not sure if this is portable for all archs and I'd use #define at the top.
> 
> ...
>> +static struct tst_test test = {
>> +	.test = run,
>> +	.setup = setup,
>> +	.options = opt,
>> +	.needs_root = 1,
>> +	.needs_tmpdir = 1,
>> +	.tcnt = NB_FUNC,
>> +};
> 
> struct tst_test test should be defined in float*.c tests, not in this
> common file included by tests.
> 
> Kind regards,
> Petr
> 
Hi Petr,

Thanks for you review,I will change the code as you said later.

Best Regards.

Hongchen Zhang