<!DOCTYPE html><html><head><title></title><style type="text/css">
p.MsoNormal,p.MsoNoSpacing{margin:0}
p.MsoNormal,p.MsoNoSpacing{margin:0}</style></head><body><div>You are welcome, and thanks for your contribution!<br></div><div><br></div><div>I suggest you the following:<br></div><div><br></div><div>- p_feed->p_items[p_feed->i_items-1].psz_title = (char *)&s_init_title[0]; <br></div><div>+ p_feed->p_items[p_feed->i_items-1].psz_title = (char *)s_init_title;<br></div><div><br></div><div>Same for the other part of the code.<br></div><div><br></div><div>Best,<br></div><div>Thomas<br></div><div><br></div><div>On Wed, Jul 22, 2020, at 22:49, Daniel Glaas wrote:<br></div><blockquote type="cite" id="qt" style=""><p>Hi Thomas,<br></p><p>I really appreciate your patient review, it hopefully made my
      patch to something that contributes to VLC and I also learned a
      lot about handling of chars and char arrays in C.<br></p><p>I attached the corrected version of the patch.<br></p><div class="qt-moz-cite-prefix">Am 22.07.20 um 08:45 schrieb Thomas
      Guillem:<br></div><blockquote type="cite" cite="mid:60492160-feea-4b7d-a867-95d61efe8c00@www.fastmail.com"><div>Hello,<br></div><div><br></div><div>Yes the cast is correct but &s_init_title[0] ==
        s_init_title, so you don't need &[0].<br></div><div><br></div><div>On Tue, Jul 21, 2020, at 21:47, Daniel Glaas wrote:<br></div><blockquote type="cite" id="qt-qt" style=""><p>Please find attached my reworked commit, I think I
          implemented all your remarks.<br></p><p>I added some casts to cast away the const qualifier from
          s_init_title to avoid that gcc prints warnings about that. I
          don't know whether this is a good style in your opinion or
          not.<br></p><div class="qt-qt-moz-cite-prefix">Am 21.07.20 um 11:20 schrieb
          Thomas Guillem:<br></div><blockquote type="cite" cite="mid:90e9f0a3-93a9-44e9-8b0a-db82b8252e35@www.fastmail.com"><div>Your commit looks good. Few remarks:<br></div><div><br></div><div>- You don't need p_initTitle, you can use initTitle
            directly<br></div><div><br></div><div>- static char initTitle[] = ""; <- missing const
            qualifier, the name doesn't respect the codestyle of the
            file (camal case), call it s_init_title (s for static).<br></div><div><br></div><div>- I don't think the comment is needed.<br></div><div><br></div><div>Best,<br></div><div>Thomas<br></div><div><br></div><div><br></div><div><br></div><div>On Tue, Jul 21, 2020, at 01:36, Daniel Glaas wrote:<br></div><blockquote type="cite" id="qt-qt-qt" style=""><div>Hello Thomas,<br></div><div><br></div><div>thank you for your review and you feedback, you pointed
              on some very new <br></div><div>aspects for me ...<br></div><div><br></div><div>I implemented the second alternative solution you
              proposed, by <br></div><div>introducing a special static const string with is
              assigned during <br></div><div>initialization of the variables and against which
              comparisons are done. <br></div><div>The way I coded this in line 75f. is surely not the
              best was, after <br></div><div>trying several minutes around, I was not able to find a
              better solution <br></div><div>that doesn't rise compiler warnings. I appreciate your
              suggestions.<br></div><div><br></div><div>And of course you are right, the strdup("") calls lead
              to memory leaks <br></div><div>as I never freed them. I'm sorry that I didn't notice
              that, thanks for <br></div><div>noticing this.<br></div><div><br></div><div>Am 20.07.20 um 08:33 schrieb Thomas Guillem:<br></div><div>> Hello,<br></div><div>><br></div><div>> On Sat, Jul 18, 2020, at 12:47, Daniel Glaas
              wrote:<br></div><div>>> In principle, looking over all possible use
              cases of strdup I agree that<br></div><div>>> there might be case where strdup() returns
              NULL. But here I used it with<br></div><div>>> an empty string as parameter, meaning that
              malloc() only needs to<br></div><div>>> allocate one single byte in the memory. If
              this really fails, then there<br></div><div>>> are other problems than only a wrong display
              of an RSS text overlay.<br></div><div>> We require all allocations to be checked and
              handled. This is not the <br></div><div>> case for every C programs and libs, but it is in
              VLC.<br></div><div>><br></div><div>><br></div><div>>> I also tried to add catches to the NULL
              pointers at those places where<br></div><div>>> they are used. But in my opinion, this makes
              the places of usage very<br></div><div>>> bad to read with a lot of NULL checks at
              various places. A lot of places<br></div><div>>> in the *Filter() function rely on valid
              pointers where the titles are<br></div><div>>> stored.<br></div><div>>><br></div><div>> I see 2 other choices :<br></div><div>> - fix psz_title NULLITY check everywhere<br></div><div>> - Assign it to a special static const string
              (likely "") : in that <br></div><div>> case, you should check that the pointer is not
              equals to this special <br></div><div>> string pointer before freeing it.<br></div><div>><br></div><div>> Also, your patch seems to leak the "" allocation
              when psz_title is <br></div><div>> replaced by a new string.<br></div><div>><br></div><div>>> Am 18.07.20 um 09:41 schrieb Rémi
              Denis-Courmont:<br></div><div>>>> Le lauantaina 18. heinäkuuta 2020, 5.07.00
              EEST Daniel Glaas a écrit :<br></div><div>>>>> Initialize title strings with non NULL
              pointer<br></div><div>>>>><br></div><div>>>>> If no valid title could be parsed out
              of an RSS feed, the <br></div><div>>>>> segmentation fault<br></div><div>>>>> caused VLC to crash. The reason
              therefore was that the expected <br></div><div>>>>> pointers to<br></div><div>>>>> the char buffers containing the titles
              were NULL pointers.<br></div><div>>>> This won't work because strdup() can
              return NULL. Better handle the NULL<br></div><div>>>> values correctly.<br></div><div>>>><br></div><div>>>
              _______________________________________________<br></div><div>>> vlc-devel mailing list<br></div><div>>> To unsubscribe or modify your subscription
              options:<br></div><div>>> <a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div><div>> _______________________________________________<br></div><div>> vlc-devel mailing list<br></div><div>> To unsubscribe or modify your subscription
              options:<br></div><div>> <a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div><div><br></div><div><br></div><div><br></div><div>_______________________________________________<br></div><div>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div><div><br></div><div><b>Attachments:</b><br></div><ul><li>0003-Initialize-title-strings-with-static-const-empty-cha.patch<br></li></ul></blockquote><div><br></div><div><br></div><pre class="qt-qt-moz-quote-pre">_______________________________________________
vlc-devel mailing list
To unsubscribe or modify your subscription options:
<a class="qt-qt-moz-txt-link-freetext" href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a>
<br></pre></blockquote><div>_______________________________________________<br></div><div>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div><div><br></div><div><b>Attachments:</b><br></div><ul><li>0003-Initialize-title-strings-with-static-const-empty-cha.patch<br></li></ul></blockquote><div><br></div><div><br></div><pre class="qt-moz-quote-pre">_______________________________________________
vlc-devel mailing list
To unsubscribe or modify your subscription options:
<a class="qt-moz-txt-link-freetext" href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></pre></blockquote><div>_______________________________________________<br></div><div>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div><div><br></div><div><b>Attachments:</b><br></div><ul><li>0003-Initialize-title-strings-with-static-const-empty-cha.patch<br></li></ul></blockquote><div><br></div></body></html>