Re: [FFmpeg-devel] [PATCH 1/2] avfilter/signature_lookup: dont leave uncleared pointers in sll_free()

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 2188 bytes --]

On Tue, Feb 06, 2024 at 11:36:13AM +0100, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > On Mon, Feb 05, 2024 at 12:51:57PM +0100, Andreas Rheinhardt wrote:
> >> Michael Niedermayer:
> >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> >>> ---
> >>>  libavfilter/signature_lookup.c | 21 ++++++++++-----------
> >>>  1 file changed, 10 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/libavfilter/signature_lookup.c b/libavfilter/signature_lookup.c
> >>> index 86dd0c66754..52a97e1bc7e 100644
> >>> --- a/libavfilter/signature_lookup.c
> >>> +++ b/libavfilter/signature_lookup.c
> >>> @@ -37,6 +37,15 @@
> >>>  #define STATUS_END_REACHED 1
> >>>  #define STATUS_BEGIN_REACHED 2
> >>>  
> >>> +static void sll_free(MatchingInfo **sll)
> >>> +{
> >>> +    while (*sll) {
> >>> +        MatchingInfo *tmp = *sll;
> >>> +        *sll = (*sll)->next;
> >>> +        av_free(tmp);
> >>> +    }
> >>
> >> This does not clear the pointers at all. This does (and avoids
> >> indirections).
> >>
> >> static void sll_free(MatchingInfo **sllp)
> >> {
> >>     MatchingInfo *sll = *sllp;
> >>
> >>     *sllp = NULL;
> >>     while (sll) {
> >>         MatchingInfo *tmp = sll;
> >>         sll = sll->next;
> >>         av_free(tmp);
> >>     }
> >> }
> > 
> > I tried it with code below, but your code is not different from mine in behavior just more complex
> > 
> 
> Your code indeed resets the pointer; it overwrites the pointer once per
> loop iteration and so sets it to NULL in the last iteration. I somehow
> overlooked that.
> I actually consider your code more complex (my code resets the original
> pointer and directly traverses the list, your code does the same, but in
> between it overwrites the original pointer to store the next pointer
> instead of using a simple stack variable for this purpose).

> Apply as you wish.

ok


[...]
> sll_free_n2() is not recursive.

the function is cursed, noone can implement it without bugs

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".