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.
- 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.
+ if (!av_strncasecmp(start, "boundary=", 9)) {
+ start += 9;
It has already be pointed out: av_stristart() to avoid the duplicated magic
number.
I've pondered the change, but with
if (!av_stristart(start, "boundary=")) {
start += 9;
'9' seems a bit random, and with
if (!av_stristart(start, "boundary=")) {
start += strlen("boundary=");
we end up with unnecessarily taking strlen at least twice.
Again, not critical, but IMHO as written it is the best compromise
between readability and efficiency.
Thanks for your comments!
Can not comment on the functional aspect, sorry.
Regards,
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
