[FFmpeg-devel] [GSoC] Proof-of-concept HTTP Server

[Nicolas George]

Le nonidi 9 germinal, an CCXXIII, Stephan Holljes a écrit :
> I hope this addresses the issues mentioned.
> I added a new label in case of failure in http_open() in favor of
> duplicated code (i.e. calling av_dict_free() multiple times). I copied
> the style from the other functions.
> 
> Signed-off-by: Stephan Holljes <klaxa1337 at googlemail.com>

The patch looks good to me (acronym to learn: LGTM) as is or with the code
moved into a separate function point.

Except one more point that was not addressed earlier: if you want to add
comments to a Git patch email, you need to put them between the "---" and
the first "diff --git" lines. That is where the small stats "2 files
changed, 28 insertions(+), 1 deletion(-)" is already inserted, it is
considered as a simple comment too.

If you put them before the "---" line, it becomes part of the commit message
when the patch is applied from the mail. For the final version, maybe better
attach the patch or push it to a public clone.

At this point, I suspect you can consider this will be applied soon (better
give it maybe two days before asking for merging) and, if you want to
strengthen your application, make the patch even better, for example getting
it to work for input as well as output.

For that, I suspect it will be convenient for you to isolate the code in its
own function, that makes handling the variables and the failure code path
easier, so you probably better follow wm4's advice.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150330/f581b384/attachment.asc>