* [ltt-dev] [UST PATCH] Fix return value handling for libustcmd
@ 2010-09-07 20:26 David Goulet
2010-09-07 20:56 ` Mathieu Desnoyers
0 siblings, 1 reply; 3+ messages in thread
From: David Goulet @ 2010-09-07 20:26 UTC (permalink / raw)
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>
---
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
^ permalink raw reply [flat|nested] 3+ messages in thread* [ltt-dev] [UST PATCH] Fix return value handling for libustcmd
2010-09-07 20:26 [ltt-dev] [UST PATCH] Fix return value handling for libustcmd David Goulet
@ 2010-09-07 20:56 ` Mathieu Desnoyers
2010-09-07 22:13 ` David Goulet
0 siblings, 1 reply; 3+ messages in thread
From: Mathieu Desnoyers @ 2010-09-07 20:56 UTC (permalink / raw)
* 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.
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
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 3+ messages in thread* [ltt-dev] [UST PATCH] Fix return value handling for libustcmd
2010-09-07 20:56 ` Mathieu Desnoyers
@ 2010-09-07 22:13 ` David Goulet
0 siblings, 0 replies; 3+ messages in thread
From: David Goulet @ 2010-09-07 22:13 UTC (permalink / raw)
-----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-----
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-09-07 22:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-07 20:26 [ltt-dev] [UST PATCH] Fix return value handling for libustcmd David Goulet
2010-09-07 20:56 ` Mathieu Desnoyers
2010-09-07 22:13 ` David Goulet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox