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

[wm4]

On Mon, 30 Nov 2015 07:27:14 -0500
Alexander Agranovsky <alex at sighthound.com> wrote:

> On 11/30/15 2:41 AM, Nicolas George wrote:
> > 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;  
> That works too.
> 
> >  
> >> +    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.  
> Without getting into a religious debate, not my favorite part of the 
> style ;)
> Will change the code to match the style of existing, though.
> 
> >> +                "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.  
> 
> As pointed later in the thread, lu is used elsewhere. And yes, MS makes 
> it interesting in this case.

No, I meant %zu. lu really isn't safe. It will fail on 64 bit Windows,
for one. (But what Nicolas meant was that %zu could fail on Windows
because Windows is stuck in the past, but we work that around if
necessary - I think.)

> >> -            size = parse_content_length(value);
> >> -            if (size < 0)
> >> -                return size;
> >> +            *size = parse_content_length(value);  
> > Did you remove the check on purpose?  
> 
> Yes. Invalid Content-Length, just as no Content-Length at all, should 
> not prevent us from processing the part.

Probably not a good idea to ignore when the server sends us definitely
broken data. I'd prefer failure or at least an error message.