<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On 9 December 2014 at 17:49, Derek Foreman <span dir="ltr"><<a href="mailto:derekf@osg.samsung.com" target="_blank">derekf@osg.samsung.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 05/12/14 06:49 AM, Marek Chalupa wrote:<br>
> There were unchecked malloc and free of conditionally allocated<br>
> memory without checking if the memory was really allocated.<br>
> Also simplify error handling in one function.<br>
><br>
> Signed-off-by: Marek Chalupa <<a href="mailto:mchqwerty@gmail.com">mchqwerty@gmail.com</a>><br>
> ---<br>
>  src/screenshooter.c | 27 +++++++++++++++++++--------<br>
>  1 file changed, 19 insertions(+), 8 deletions(-)<br>
><br>
> diff --git a/src/screenshooter.c b/src/screenshooter.c<br>
> index 6246cda..b7b1fd0 100644<br>
> --- a/src/screenshooter.c<br>
> +++ b/src/screenshooter.c<br>
> @@ -464,8 +464,11 @@ weston_recorder_free(struct weston_recorder *recorder)<br>
>  {<br>
>       if (recorder == NULL)<br>
>               return;<br>
> +<br>
> +     if (recorder->tmpbuf)<br>
> +             free(recorder->tmpbuf);<br>
> +<br>
>       free(recorder->rect);<br>
> -     free(recorder->tmpbuf);<br>
<br>
</span>I think recorder->tmpbuf should be NULL if it was never required<br>
(recorder is allocated with zalloc), and free(NULL); is a NOP...<br></blockquote><div><br></div><div>Yep, man pages say that free(NULL) is a no-op.<br>Until now I've thought that NULL is invalid arg for free()... Man keeps learning :)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
>       free(recorder->frame);<br>
>       free(recorder);<br>
>  }<br>
> @@ -495,12 +498,16 @@ weston_recorder_create(struct weston_output *output, const char *filename)<br>
><br>
>       if ((recorder->frame == NULL) || (recorder->rect == NULL)) {<br>
>               weston_log("%s: out of memory\n", __func__);<br>
> -             weston_recorder_free(recorder);<br>
> -             return;<br>
> +             goto err_recorder;<br>
>       }<br>
><br>
> -     if (!do_yflip)<br>
> +     if (!do_yflip) {<br>
>               recorder->tmpbuf = malloc(size);<br>
> +             if (recorder->tmpbuf == NULL) {<br>
> +                     weston_log("%s: out of memory\n", __func__);<br>
> +                     goto err_recorder;<br>
> +             }<br>
<br>
</span>Yup, we need this bit.<br>
<br>
And I like the goto err_recorder stuff as well, though I guess that's a<br>
matter of preference.<br>
<div class="HOEnZb"><div class="h5"><br>
> +     }<br>
><br>
>       header.magic = WCAP_HEADER_MAGIC;<br>
><br>
> @@ -514,8 +521,7 @@ weston_recorder_create(struct weston_output *output, const char *filename)<br>
>               break;<br>
>       default:<br>
>               weston_log("unknown recorder format\n");<br>
> -             weston_recorder_free(recorder);<br>
> -             return;<br>
> +             goto err_recorder;<br>
>       }<br>
><br>
>       recorder->fd = open(filename,<br>
> @@ -523,8 +529,7 @@ weston_recorder_create(struct weston_output *output, const char *filename)<br>
><br>
>       if (recorder->fd < 0) {<br>
>               weston_log("problem opening output file %s: %m\n", filename);<br>
> -             weston_recorder_free(recorder);<br>
> -             return;<br>
> +             goto err_recorder;<br>
>       }<br>
><br>
>       header.width = output->current_mode->width;<br>
> @@ -535,6 +540,12 @@ weston_recorder_create(struct weston_output *output, const char *filename)<br>
>       wl_signal_add(&output->frame_signal, &recorder->frame_listener);<br>
>       output->disable_planes++;<br>
>       weston_output_damage(output);<br>
> +<br>
> +     return;<br>
> +<br>
> +err_recorder:<br>
> +     weston_recorder_free(recorder);<br>
> +     return;<br>
>  }<br>
><br>
>  static void<br>
><br>
<br></div></div></blockquote><div><br></div><div>Thanks for reviewing, sending new patch with the free() thing fixed<br><br></div><div>Marek <br></div></div><br></div></div>