[FFmpeg-devel] [PATCH] avcodec/v4l2_m2m: Fix segmentation fault due to a missing NULL check in v4l2_context.c

[FFmpeg-devel] [PATCH] avcodec/v4l2_m2m: Fix segmentation fault due to a missing NULL check in v4l2_context.c

[Eslam Samy via ffmpeg-devel]

---
 libavcodec/v4l2_context.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
index be1df3785b..ea11a689bc 100644
--- a/libavcodec/v4l2_context.c
+++ b/libavcodec/v4l2_context.c
@@ -329,7 +329,7 @@ start:
         /* if we are trying to get free buffers but none have been queued yet,
          * or if no buffers have been allocated yet, no need to raise a warning
          */
-        if (timeout == 0) {
+        if ((timeout == 0) && (ctx->buffers != NULL)) {
             if (!ctx->buffers)
                 return NULL;
 
-- 
2.43.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/v4l2_m2m: Fix segmentation fault due to a missing NULL check in v4l2_context.c

[Nicolas George]

Thanks for the patch.

Eslam Samy via ffmpeg-devel (HE12025-02-18):
> ---
>  libavcodec/v4l2_context.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
> index be1df3785b..ea11a689bc 100644
> --- a/libavcodec/v4l2_context.c
> +++ b/libavcodec/v4l2_context.c
> @@ -329,7 +329,7 @@ start:
>          /* if we are trying to get free buffers but none have been queued yet,
>           * or if no buffers have been allocated yet, no need to raise a warning
>           */
> -        if (timeout == 0) {

> +        if ((timeout == 0) && (ctx->buffers != NULL)) {

This does not match ffmpeg's coding style, the extra parentheses and the
!= NULL.

>              if (!ctx->buffers)
>                  return NULL;

Can you explain how this hunk makes sense with your change?

Regards,

-- 
  Nicolas George
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/v4l2_m2m: Fix segmentation fault due to a missing NULL check in v4l2_context.c

[Eslam Samy via ffmpeg-devel]

 
Hi Nicolas,

Sorry for the misunderstanding—this is my first patch submitted to FFmpeg, and I'm still familiarizing myself with the codebase.

The logic behind the change I made is explained here: Ticket #9722. I tried to fix the issue by adding a null check to prevent the segmentation fault when ctx->buffers is accessed before being initialized.

Regarding the coding style, I reviewed the FFmpeg developer documentation, but I couldn't find any explicit rules on this matter. However, I completely understand the importance of adhering to FFmpeg's coding standards, and I would be happy to modify the patch to align with the style you suggest.

Thanks for your feedback, and I look forward to your guidance!

Best regards,
Eslam
    On Tuesday, February 18, 2025 at 01:27:49 PM GMT+2, Nicolas George <george@nsup.org> wrote:  
 
 Thanks for the patch.

Eslam Samy via ffmpeg-devel (HE12025-02-18):
> ---
>  libavcodec/v4l2_context.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c
> index be1df3785b..ea11a689bc 100644
> --- a/libavcodec/v4l2_context.c
> +++ b/libavcodec/v4l2_context.c
> @@ -329,7 +329,7 @@ start:
>          /* if we are trying to get free buffers but none have been queued yet,
>          * or if no buffers have been allocated yet, no need to raise a warning
>          */
> -        if (timeout == 0) {

> +        if ((timeout == 0) && (ctx->buffers != NULL)) {

This does not match ffmpeg's coding style, the extra parentheses and the
!= NULL.

>              if (!ctx->buffers)
>                  return NULL;

Can you explain how this hunk makes sense with your change?

Regards,

-- 
  Nicolas George
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
  
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/v4l2_m2m: Fix segmentation fault due to a missing NULL check in v4l2_context.c

[Nicolas George]

Eslam Samy via ffmpeg-devel (HE12025-02-18):
> Hi Nicolas,

@anchor{What is top-posting?}
@section What is top-posting?

See @url{https://en.wikipedia.org/wiki/Posting_style#Top-posting}.

Instead, use trimmed interleaved/inline replies (@url{https://lists.ffmpeg.org/pipermail/ffmpeg-user/2017-April/035849.html, example}).

> The logic behind the change I made is explained here: Ticket #9722. I
> tried to fix the issue by adding a null check to prevent the
> segmentation fault when ctx->buffers is accessed before being
> initialized.

Can you confirm that your patch really fixes the issue? You need to
confirm you have tested two things: (1) the issue does happen with the
current code base and (2) the issue no longer happens with the patch.

I strongly suggest you neglected (1).

> Regarding the coding style, I reviewed the FFmpeg developer
> documentation, but I couldn't find any explicit rules on this matter.
> However, I completely understand the importance of adhering to
> FFmpeg's coding standards, and I would be happy to modify the patch to
> align with the style you suggest.

@item
No unnecessary parentheses, unless it helps readability:

For the rest, you are supposed to imitate the existing code. Feel free
to propose adding the rule about null and 0 comparisons.

Regards,

-- 
  Nicolas George
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".