[ltt-dev] [UST PATCH] Fix return value handling for libustcmd

[david.goulet@polymtl.ca (David Goulet)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



On 10-09-07 04:56 PM, Mathieu Desnoyers wrote:
> * David Goulet (david.goulet at polymtl.ca) wrote:
>> Return value was not checked correctly so this was
>> triggering a free() on an invalid pointer causing
>> ustctl to fail badly.
>>
>> Signed-off-by: David Goulet <david.goulet at polymtl.ca>
> 
> Acked-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> 
> The fact that Pierre-Marc, Douglas and Philippe all got it wrong in the
> function return value description/implementation supports my
> recommandation to switch 
> 
> send_message_fd()
> 
> along with *all* its callers to the following returns values:
> 
> All negative values: Error returned by patient_send().
>                      (includes -EPIPE: connection closed)
> 0: success.
> 
> Side-note: write_current_subbuffer() and on_read_partial_subbuffer() in
> ustd/ustd.c use patient_write(), but the manpage states that the
> returns values for the "0" case is different between files and
> non-files (e.g. unix sockets). So we should probably look into this more
> closely.
> 

I agree. The return value 1 for success is quite "unintuitive". I did follow
your discussion on that and I have to agree with you on that.

David

> Thanks,
> 
> Mathieu
> 
>> ---
>>  libustcmd/ustcmd.c |   11 ++++-------
>>  1 files changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/libustcmd/ustcmd.c b/libustcmd/ustcmd.c
>> index 5b4fd02..f0a6ae0 100644
>> --- a/libustcmd/ustcmd.c
>> +++ b/libustcmd/ustcmd.c
>> @@ -97,7 +97,7 @@ int ustcmd_set_marker_state(const char *mn, int state, pid_t pid)
>>  	}
>>  
>>  	result = ustcmd_send_cmd(cmd, pid, NULL);
>> -	if (result) {
>> +	if (result != 1) {
>>  		free(cmd);
>>  		return USTCMD_ERR_GEN;
>>  	}
>> @@ -182,9 +182,8 @@ int ustcmd_get_subbuf_size(const char *channel, pid_t pid)
>>  	}
>>  
>>  	result = ustcmd_send_cmd(cmd, pid, &reply);
>> -	if (result) {
>> +	if (result != 1) {
>>  		free(cmd);
>> -		free(reply);
>>  		return -1;
>>  	}
>>  
>> @@ -214,9 +213,8 @@ int ustcmd_get_subbuf_num(const char *channel, pid_t pid)
>>  	}
>>  
>>  	result = ustcmd_send_cmd(cmd, pid, &reply);
>> -	if (result) {
>> +	if (result != 1) {
>>  		free(cmd);
>> -		free(reply);
>>  		return -1;
>>  	}
>>  
>> @@ -488,7 +486,6 @@ int ustcmd_get_sock_path(char **sock_path, pid_t pid)
>>  	result = ustcmd_send_cmd(cmd, pid, &reply);
>>  	if (result != 1) {
>>  		free(cmd);
>> -		free(reply);
>>  		return USTCMD_ERR_GEN;
>>  	}
>>  
>> @@ -516,7 +513,7 @@ int ustcmd_force_switch(pid_t pid)
>>   * @param pid	Targeted PID
>>   * @param reply	Pointer to string to be filled with a reply string (must
>>   *		be NULL if no reply is needed for the given command).
>> - * @return	-1 if successful, 0 on EOT, 1 on success
>> + * @return	-1 if not successful, 0 on EOT, 1 on success
>>   */
>>  
>>  int ustcmd_send_cmd(const char *cmd, const pid_t pid, char **reply)
>> -- 
>> 1.7.2.2
>>
> 

- -- 
David Goulet
LTTng project, DORSAL Lab.

1024D/16BD8563
BE3C 672B 9331 9796 291A  14C6 4AF7 C14B 16BD 8563
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAkyGuO0ACgkQSvfBSxa9hWM5AwCeIF6iGP2Dv7U1xbwXbmUJa3Ad
ijUAniY6RlvMrJe4ZRbfbzYKBVJS4SdE
=Sd9r
-----END PGP SIGNATURE-----