[v4,1/5] libs/libltpnewipc/libnewipc.c: Add msg_do_reader/msg_do_writer function

Copy old libmsgctl.c three functions(doreader,dowriter,verify) into libnewipc.c.
It is prepared for upcoming new api msgstress case. The reason for not using a new
c lib file(ie libnewmsgctl.c) is that current libnewipc is small(only has getipckey,
get_used_queues, probe_free_addr, get_ipc_timestamp function). So put them into libnewipc.c.
Also use tst_brk for failure because we don't want to print many info in stress test when
case fails.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 include/libnewipc.h           | 11 +++++
 libs/libltpnewipc/libnewipc.c | 75 +++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+)

Hi!
First of all sorry for the long delay.

> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> ---
>  include/libnewipc.h           | 11 +++++
>  libs/libltpnewipc/libnewipc.c | 75 +++++++++++++++++++++++++++++++++++
>  2 files changed, 86 insertions(+)
> 
> diff --git a/include/libnewipc.h b/include/libnewipc.h
> index 075364f85..0f099c939 100644
> --- a/include/libnewipc.h
> +++ b/include/libnewipc.h
> @@ -45,6 +45,14 @@
>  #define INT_SIZE	4
>  #define MODE_MASK	0x01FF
>  
> +struct mbuffer {
> +	long type;
> +	struct {
> +		char len;
> +		char pbytes[99];
> +	} data;
> +};
> +
>  key_t getipckey(const char *file, const int lineno);
>  #define GETIPCKEY() \
>  	getipckey(__FILE__, __LINE__)
> @@ -59,4 +67,7 @@ void *probe_free_addr(const char *file, const int lineno);
>  
>  time_t get_ipc_timestamp(void);
>  
> +void msg_do_reader(long key, int tid, long type, int child, int nreps);
> +
> +void msg_do_writer(long key, int tid, long type, int child, int nreps);
>  #endif /* newlibipc.h */
> diff --git a/libs/libltpnewipc/libnewipc.c b/libs/libltpnewipc/libnewipc.c
> index d0974bbe0..09871b421 100644
> --- a/libs/libltpnewipc/libnewipc.c
> +++ b/libs/libltpnewipc/libnewipc.c
> @@ -99,3 +99,78 @@ time_t get_ipc_timestamp(void)
>  
>  	return ts.tv_sec;
>  }
> +
> +static int verify(char *buf, char val, int size, int child)
> +{
> +	while (size-- > 0) {
> +		if (*buf++ != val) {
> +			tst_res(TFAIL,
> +				"Verify error in child %d, *buf = %x, val = %x, size = %d",
> +				child, *buf, val, size);

Actually this piece of code had a bug in the original version as well,
as we do *buf++ we end up one byte after the position we wanted to
print if we ever got wrong byte, possibly out of the buffer as well.

So I guess that this will be much better and easier to read with usuall
for loop and array subscript:

	for (i = 0; i < size; i++) {
		if (buf[i] != val)  {
		...

Also we report failure in the msg_do_reader() so I guess that it would
be slightly better to report TINFO with the details here and let the
msg_do_reader() report the failure.

> +			return 1;
> +		}
> +	}
> +	return 0;
> +}
> +
> +void msg_do_reader(long key, int tid, long type, int child, int nreps)
> +{
> +	int i, size;
> +	int id;
> +	struct mbuffer buffer;
> +
> +	id = SAFE_MSGGET(key, 0);
> +	if (id != tid) {
> +		tst_brk(TFAIL,
> +			"Message queue mismatch in the reader of child group %d for message queue id %d ",
> +			child, id);
> +	}
> +	for (i = 0; i < nreps; i++) {
> +		memset(&buffer, 0, sizeof(buffer));
> +
> +		size = SAFE_MSGRCV(id, &buffer, 100, type, 0);
> +		if (buffer.type != type) {
> +			tst_brk(TFAIL,
> +				"Type mismatch in child %d, read #%d, for message got %ld, exected %ld",
> +				child, (i + 1), buffer.type, type);
> +		}
> +		if (buffer.data.len + 1 != size) {
> +			tst_brk(TFAIL,
> +				"Size mismatch in child %d, read #%d, for message got %d, expected %d",
> +				child, (i + 1), buffer.data.len + 1, size);
> +		}
> +		if (verify(buffer.data.pbytes, (key % 255), size - 1, child)) {
> +			tst_brk(TFAIL,
> +				"Verify failed in child %d read # = %d, key = %lx",
> +				child, (i + 1), key);
> +		}
> +		key++;
> +	}
> +}
> +
> +void msg_do_writer(long key, int tid, long type, int child, int nreps)
> +{
> +	int i, size;
> +	int id;
> +	struct mbuffer buffer;
> +
> +	id = SAFE_MSGGET(key, 0);
> +	if (id != tid) {
> +		tst_brk(TFAIL,
> +			"Message queue mismatch in the writer of child group %d for message queue id %d",
> +			child, id);
> +	}
> +
> +	for (i = 0; i < nreps; i++) {
> +		memset(&buffer, 0, sizeof(buffer));

We set the relevant part of the buffer with (key % 255), do we really
have to clear it here?

> +		do {
> +			size = (lrand48() % 99);
> +		} while (size == 0);
> +		memset(buffer.data.pbytes, (key % 255), size);
> +		buffer.data.len = size;
> +		buffer.type = type;
> +		SAFE_MSGSND(id, &buffer, size + 1, 0);
> +		key++;
> +	}
> +}

Hi!
> +void msg_do_reader(long key, int tid, long type, int child, int nreps)
> +{
> +	int i, size;
> +	int id;
> +	struct mbuffer buffer;
> +
> +	id = SAFE_MSGGET(key, 0);
> +	if (id != tid) {
> +		tst_brk(TFAIL,
> +			"Message queue mismatch in the reader of child group %d for message queue id %d ",
> +			child, id);
> +	}

Also looking at how these functions are used, we fork a child that calls
this function then wait it and ignore the exit value it does not make
sense to use tst_brk() here at all.

The tst_brk() is supposed to exit the whole test, including all
suprocesses but that does not work if we throw away the child return
value. This piece of code looks like we exit the whole test here, which
isn't simply true.

So either we should do tst_res(TFAIL, ...) followed by an exit(0) here,
or we should handle the return value in the parent. I.e. stop the test
if one of the children reported a failure.

> +	for (i = 0; i < nreps; i++) {
> +		memset(&buffer, 0, sizeof(buffer));
> +
> +		size = SAFE_MSGRCV(id, &buffer, 100, type, 0);
> +		if (buffer.type != type) {
> +			tst_brk(TFAIL,
> +				"Type mismatch in child %d, read #%d, for message got %ld, exected %ld",
> +				child, (i + 1), buffer.type, type);
> +		}
> +		if (buffer.data.len + 1 != size) {
> +			tst_brk(TFAIL,
> +				"Size mismatch in child %d, read #%d, for message got %d, expected %d",
> +				child, (i + 1), buffer.data.len + 1, size);
> +		}
> +		if (verify(buffer.data.pbytes, (key % 255), size - 1, child)) {
> +			tst_brk(TFAIL,
> +				"Verify failed in child %d read # = %d, key = %lx",
> +				child, (i + 1), key);
> +		}
> +		key++;
> +	}

It would also make sense to do exit(0); here so that we do not have to
repeat that in each test.

> +}
> +
> +void msg_do_writer(long key, int tid, long type, int child, int nreps)
> +{
> +	int i, size;
> +	int id;
> +	struct mbuffer buffer;
> +
> +	id = SAFE_MSGGET(key, 0);
> +	if (id != tid) {
> +		tst_brk(TFAIL,
> +			"Message queue mismatch in the writer of child group %d for message queue id %d",
> +			child, id);
> +	}
> +
> +	for (i = 0; i < nreps; i++) {
> +		memset(&buffer, 0, sizeof(buffer));
> +
> +		do {
> +			size = (lrand48() % 99);
> +		} while (size == 0);
> +		memset(buffer.data.pbytes, (key % 255), size);
> +		buffer.data.len = size;
> +		buffer.type = type;
> +		SAFE_MSGSND(id, &buffer, size + 1, 0);
> +		key++;
> +	}
> +}

And same here.