* David Goulet ([email protected]) 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 <[email protected]>

Acked-by: Mathieu Desnoyers <[email protected]>

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

_______________________________________________
ltt-dev mailing list
[email protected]
http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev

Reply via email to