> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of > Andreas Rheinhardt > Sent: Mittwoch, 16. April 2025 08:31 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH 01/12] > fftools/textformat/avtextformat: Simplify avtext_print_rational() > > softworkz .: > > > > > >> -----Original Message----- > >> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of > >> Andreas Rheinhardt > >> Sent: Mittwoch, 16. April 2025 07:37 > >> To: ffmpeg-devel@ffmpeg.org > >> Subject: Re: [FFmpeg-devel] [PATCH 01/12] > >> fftools/textformat/avtextformat: Simplify avtext_print_rational() > >> > >> softworkz .: > >>> > >>> > >>>> -----Original Message----- > >>>> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of > >>>> Andreas Rheinhardt > >>>> Sent: Mittwoch, 16. April 2025 06:28 > >>>> To: ffmpeg-devel@ffmpeg.org > >>>> Subject: Re: [FFmpeg-devel] [PATCH 01/12] > >>>> fftools/textformat/avtextformat: Simplify avtext_print_rational() > >>>> > >>>> softworkz .: > >>>>> > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf > Of > >>>>>> Andreas Rheinhardt > >>>>>> Sent: Dienstag, 15. April 2025 10:36 > >>>>>> To: ffmpeg-devel@ffmpeg.org > >>>>>> Subject: Re: [FFmpeg-devel] [PATCH 01/12] > >>>>>> fftools/textformat/avtextformat: Simplify > avtext_print_rational() > >>>>>> > >>>>>> softworkz .: > >>>>>>> > >>>>>>> > >>>>>>>> -----Original Message----- > >>>>>>>> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On > Behalf > >> Of > >>>>>>>> Andreas Rheinhardt > >>>>>>>> Sent: Dienstag, 15. April 2025 03:00 > >>>>>>>> To: FFmpeg development discussions and patches <ffmpeg- > >>>>>>>> de...@ffmpeg.org> > >>>>>>>> Subject: [FFmpeg-devel] [PATCH 01/12] > >>>>>> fftools/textformat/avtextformat: > >>>>>>>> Simplify avtext_print_rational() > >>>>>>>> > >>>>>>>> Patches attached. > >>>>>>>> > >>>>>>>> - Andreas > >>>>>>> > >>>>>>> > >>>>>>> Hi Andreas, > >>>>>>> > >>>>>>> thanks a lot for working through this. I'll go over it > tomorrow. > >>>>>>> > >>>>>>> As to not waste your time, it's probably best when we get > those > >>>>>>> changes applied in a timely manner so that I can rebase the > new > >>>>>>> patchset on top of it. > >>>>>>> > >>>>>>> Since you're sending the patches as attachments: > >>>>>>> How do you want me to reply with code context? Whole files or > >>>>>>> just snippets? And quoted? > >>>>>>> > >>>>>> > >>>>>> Snippets is better. So is quoted. > >>>>>> > >>>>>> - Andreas > >>>>>> > >>>>> > >>>>> Hi Andreas, > >>>>> > >>>>> thanks again for the well-spotted improvements. Just two notes: > >>>>> > >>>>> > >>>>> 0007-fftools-textformat-Use-av_default_item_name.patch > >>>>> > >>>>> In the new patchset, those macros are removed from the > individual > >>>>> files. There's now a single macro in tf_internal.h and I've > >> applied > >>>>> this change there. > >>>> > >>>> So you use a move/deduplicate commit to change something? Not > good. > >>> > >>> No. The patchset has a deduplication commit. That's what I've > >> submitted > >>> to the ML already. > >>> Now I made another commit locally (on top of that) which makes > this > >> change. > >>> > >>> > >>>>> 0008-fftools-textformat-avtextformat-Fix-segfault-upon-al.patch > >>>>> 0009-fftools-textformat-avtextformat-Fix-segfault-upon-al.patch > >>>>> > >>>>> Can this happen? > >>>> > >>>> Of course it can. All allocations can fail. That's why we check > >> them. > >>>> Have you been coding with the assumption that allocations never > >> fail? > >>>> (You can use av_max_alloc(1); to simulate allocation failures.) > >>> > >>> Allocations can fail, but statically initialized global const > >> values? > >> > >> The pointers to said static objects are only set after having > >> allocated > >> the private context. So the issue can happen (not with current > master) > > > > Current master is on top of which I had applied your patches and > > there's no possible way for those pointers to be null. > > > > That you are submitting a patch that is fixing a flaw in an > > unmerged future patch is pretty cool, but was also beyond my range > > of consideration. 😊 > > > > Thanks for the clarification. > > > > You completely misunderstand (or you try to be trolling me as your 😊 > indicates). "The issue" that can no longer happen with current master > is > the segfault after allocation failure. This could really happen before > my fixes. The pointers to writer/formatter can still be NULL even on > master (namely after allocation failure), therefore the checks need to > be there.
I would never troll you in such a weird way. I just looked at a very wrong place, please take my apologies for the confusion. Thanks, sw _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
