[FFmpeg-devel] [PATCH] avformat/http: clarify that ffmpeg will attempt to add missing CRLF

[Gyan]

On 29-01-2019 02:31 AM, Marton Balint wrote:
>
>
> On Mon, 28 Jan 2019, Gyan wrote:
>
>>
>>
>> On 28-01-2019 01:53 PM, Marton Balint wrote:
>>> From bc08c60761df77b37c83a4c285f3ca45e5045979 Mon Sep 17 00:00:00 2001
>>> From: Gyan Doshi <ffmpeg at gyani.pro>
>>> Date: Mon, 28 Jan 2019 12:20:02 +0530
>>> Subject: [PATCH] avformat/http: clarify that ffmpeg will attempt to add
>>>  missing CRLF
>>>
>>> ---
>>>  libavformat/http.c | 7 +++++--
>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavformat/http.c b/libavformat/http.c
>>> index 240304f6e6..2f2ce856bc 100644
>>> --- a/libavformat/http.c
>>> +++ b/libavformat/http.c
>>> @@ -541,10 +541,13 @@ static int http_open(URLContext *h, const char 
>>> *uri, int flags,
>>>          int len = strlen(s->headers);
>>>          if (len < 2 || strcmp("\r\n", s->headers + len - 2)) {
>>>              av_log(h, AV_LOG_WARNING,
>>> -                   "No trailing CRLF found in HTTP header.\n");
>>> +                   "No trailing CRLF found in HTTP header. Adding 
>>> it.\n");
>>>              ret = av_reallocp(&s->headers, len + 3);
>>> -            if (ret < 0)
>>> +            if (ret < 0) {
>>> +            av_log(h, AV_LOG_ERROR,
>>> +                   "Failed to add trailing CRLF.\n");
>>>
>>> In general I am against adding messages for ENOMEM cases.
>>
>> The codepath above is contingent upon malformed user input, and not a 
>> routine alloc.
>> Log message is extended to inform the user we're correcting the 
>> header.  So I consider it good practice to report its failure, rather 
>> than leaving it opaque.
>
> You do report failure with the returned error code. I really don't see 
> a case where it actually helps the user in any way to know that memory 
> exhausted there and not before. Therefore the "Failed to add trailing 
> CRLF" message is useless.

Ok, will remove and push.

Gyan