[flac-dev] [PATCH] for win_utf8_io.c

Janne Hyvärinen cse at sci.fi
Fri Aug 8 09:38:11 PDT 2014


Some comments for patch #1, I chose the non-secure versions because they 
are faster and produce smaller binary. The functions where these 
printings are performed can't in my opinion ever exceed the safety 
margin of 32 KB. They print short help and error texts and occasionally 
filename, which with APIs is restricted to 260 characters. And you can't 
feed it longer faulty names either because maximum command line length 
is much shorter than 32 KB.

Patch #2 is good. I was apparently not thinking when writing that.

The break that patch #3 removes is there for a reason. If there is an 
error in string conversion there's no point in continuing the operation. 
All conversions are discarded if something failed so not exiting from 
the loop is wasted effort.

On 8.8.2014 18:18, lvqcl wrote:
> For better readability the patch is divided by 3 parts.
>
> Part #1: for a bit better security replace
>     vsprintf(utmp, format, argptr)
> with
>     vsnprintf_s(utmp, 32768, _TRUNCATE, format, argptr)
>
>
> Part #2: potential memleak fixed: utf8argv[i] are not freed
> when utf8argv itself is freed.
>
>
> Part #3: 'if (ret != 0) break;' line seems redundant.
>
>
> _______________________________________________
> flac-dev mailing list
> flac-dev at xiph.org
> http://lists.xiph.org/mailman/listinfo/flac-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.xiph.org/pipermail/flac-dev/attachments/20140808/5f8724da/attachment.htm 


More information about the flac-dev mailing list