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 <ffm...@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.

Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to