Parrot-Developers/ulog

bug found in the libulogctl

Closed this issue · 4 comments

as we can see , there is the ulogctl.c(I assume it was the ulog client after compiled ) call the ulogctl_cli_set_tag_level/ulogctl_cli_set_all_level eg to set the logctrl server's log level .
I found that the the ulogctl_cli_set_all_level(...) will destroy the msg created in the function then assign the null to the msg point. that seems OK cause avoiding the memory leak.
but this msg was used in the send_cb(..) ,which will judge the msg is null or not ,then exit. so the the self->cbs.request_status will never be execute . this will lead the ulog client hung there waiting for the next loop ever and ever.
maybe it's a logical bug .

Hi, thanks for the report.

I checked rapidly the (quite old) code. I am not sure the mentioned functions have issues. It seems for me that the 'self->msg' is destroyed and cleared only in case of error. If the message has been sent, the function return early with 0 without destroying the message.

However, in the main code of the command line tool (ulogctl.c), if the call to one on these functions fails, the waiting loop is still entered and in that case no callback will wake it up I think.

Do you have a use case that fail to check further, (the error cases I can see should be very rare) ?

YMM

it seems you are right ,the msg will only be destroyed while error occured . then I rerun the server (in the example folder) and the client ,I got the message out: ulogctl-client: ulogctl: send_cb:215: err=-22(Invalid argument). it seems that the msg was destroyed indeed . if the send_cb function work right ,the main process will exit (case the s_app.stopped will be set to 1 ), Am I right .
here is my use case:

  1. compiled the server in example folder. run with the param just " -i xxx";
  2. run the client in the tool folder, with param "-a E inet:0.0.0.0:XXXX";
    then the client will stop ,any input will be invalid except the ctrl+c.

I suppose the client will exit after send the command to the server?
if I'm right , when the selft->msg deleted and set to null?

Ok I think the issue is in the asynchrnous send if the client is not yet connected ie the send_msg function. In it the message is effectively destroyed after sending the message. whereas it is not destroyed if the send failed...
so the code should be more like this:

static int send_msg(struct ulogctl_cli *self)
{
int res = 0;

RETURN_ERR_IF_FAILED(self != NULL, -EINVAL);
RETURN_ERR_IF_FAILED(self->msg != NULL, -EINVAL);

res = pomp_ctx_send_msg(self->pomp_ctx, self->msg);
if (res < 0) {
	LOG_ERRNO("pomp_ctx_send_msg", -res);

	/* Clear the message as the send failed */
	pomp_msg_destroy(self->msg);
	self->msg = NULL;
	return res;
}

/* Keep msg arround for the answer */
return 0;

}

it seems right, thanks .