Re: [LTP] [PATCH v3 01/10] Rewrite mesgq_nstest.c using new LTP API

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> +#include <sys/wait.h>
>  #include <sys/msg.h>
> -#include <libclone.h>
> -#include "test.h"
> -#include "ipcns_helper.h"
> -
> -#define KEY_VAL		154326L
> -#define UNSHARESTR	"unshare"
> -#define CLONESTR	"clone"
> -#define NONESTR		"none"
> -
> -char *TCID = "mesgq_nstest";
> -int TST_TOTAL = 1;
> -int p1[2];
> -int p2[2];
> -struct msg_buf {
> -	long int mtype;		/* type of received/sent message */
> -	char mtext[80];		/* text of the message */
> +#include <sys/types.h>
> +#include "tst_safe_sysv_ipc.h"
> +#include "tst_test.h"
> +#include "common.h"
> +
> +#define KEY_VAL 154326L
> +#define MSG_TYPE 5
> +#define MSG_TEXT "My message!"
> +
> +static char *str_op = "clone";

There is a small problem with setting the str_op here. The test library
expects that the str_* variables are initialized to NULL and uses that
logic to detect redefinitions (i.e. passing a parameter more than once
on the commandline). Which means that we get a warning in the case that
we pass -m on the commandline and because of that warning the test ends
with non-zero exit value.

I guess that the easiests solution would be not to initialize it here
and instead default to T_NONE in the get_clone_unshare_enum() when NULL
is passed.

> +static int use_clone;
> +static int ipc_id = -1;
> +
> +static struct msg_buf {
> +	long mtype;
> +	char mtext[80];
>  } msg;
>  
> -void mesgq_read(int id)
> +static int check_mesgq(LTP_ATTRIBUTE_UNUSED void *vtest)
>  {
> -	int READMAX = 80;
> -	int n;
> -	/* read msg type 5 on the Q; msgtype, flags are last 2 params.. */
> +	int id, n;
>  
> -	n = msgrcv(id, &msg, READMAX, 5, 0);
> -	if (n == -1)
> -		perror("msgrcv"), tst_exit();
> -
> -	tst_resm(TINFO, "Mesg read of %d bytes; Type %ld: Msg: %.*s",
> -		 n, msg.mtype, n, msg.mtext);
> -}
> +	id = msgget(KEY_VAL, 0);
>  
> -int check_mesgq(void *vtest)
> -{
> -	char buf[3];
> -	int id;
> +	if (id < 0) {
> +		if (use_clone == T_NONE)
> +			tst_res(TFAIL, "Plain cloned process didn't find mesgq");
> +		else
> +			tst_res(TPASS, "%s: container didn't find mesgq", str_op);
> +	} else {
> +		if (use_clone == T_NONE)
> +			tst_res(TPASS, "Plain cloned process found mesgq inside container");
> +		else
> +			tst_res(TFAIL, "%s: container init process found mesgq", str_op);
>  
> -	(void) vtest;
> +		n = SAFE_MSGRCV(id, &msg, sizeof(msg.mtext), MSG_TYPE, 0);
>  
> -	close(p1[1]);
> -	close(p2[0]);
> +		tst_res(TINFO, "Mesg read of %d bytes, Type %ld, Msg: %s", n, msg.mtype, msg.mtext);
>  
> -	read(p1[0], buf, 3);
> -	id = msgget(KEY_VAL, 0);
> -	if (id == -1)
> -		write(p2[1], "notfnd", 7);
> -	else {
> -		write(p2[1], "exists", 7);
> -		mesgq_read(id);
> +		if (strcmp(msg.mtext, MSG_TEXT))
> +			tst_res(TFAIL, "Received the wrong text message");
>  	}

We can save some indentation by using return instead of else here as:

	if (id < 0) {
		...
		return;
	}

	if (use_clone == T_NONE)
	...

	n = SAFE_MSGRCV(...);

Also it does not make sense to try to receive any messages in the case
that use_clone != T_NONE.

-- 
Cyril Hrubis
chrubis@suse.cz

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