[FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.

[Nicolas George]

Le nonidi 9 frimaire, an CCXXIV, Alexander Agranovsky a écrit :
> Please see the updated patches attached. The trimming loop that was subject
> of the discussion had been rewritten to use indices rather than pointer
> arithmetics.

This kind of drastic change was not necessary, you can do the same with
pointers. IMHO, the best way of dealing with that situation is this: when
dealing with the end of the string, have the pointer point AFTER the end of
the string, i.e.:

	char *p = string + strlen(string); // not -1
	if (p > string && av_isspace(p[-1]))
	    *(--p) = 0;

> +    char*       boundary;

Here and in all new code, please use "char *var" instead of "char* var" for
consistency. There is a good reason for that: "char* a, b" means that a is a
pointer and b is not, grouping the pointer mark with the type is misleading.

> +                "Expected boundary '%s' not found, instead found a line of %lu bytes\n",
> +                expected_boundary,
> +                strlen(line));

"%lu" is not correct for size_t. The correct type would be %zu, but it is
possible that we have to use another construct to avoid bugs from microsoft
libraries, see other instances in the code for examples.

> -            size = parse_content_length(value);
> -            if (size < 0)
> -                return size;
> +            *size = parse_content_length(value);

Did you remove the check on purpose?

> +        if (!av_strncasecmp(start, "boundary=", 9)) {
> +            start += 9;

It has already be pointed out: av_stristart() to avoid the duplicated magic
number.

Can not comment on the functional aspect, sorry.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151130/a0dfff7a/attachment.sig>