On Tue, Jun 6, 2017 at 9:22 PM, Corinna Vinschen wrote: > On Jun 6 17:57, Corinna Vinschen wrote: >> > > On Wed, May 10, 2017 at 11:30:46AM +0200, Erik Bray wrote: >> > >> [...] >> > >> The attached test demonstrates the bug. Given an output buffer of N >> > >> wide characters, wcsxfrm will cause bytes beyond the destination size >> > >> to be reversed. I believe it might actually be a bug in the underlying >> > >> LCMapStringW workhorse (this is on Windows 10; have not tested other >> > >> versions). >> > >> >> > >> According to its docs [1], the cchDest argument (size of the >> > >> destination buffer) is treated as a *byte* count when using >> > >> LCMAP_SORTKEY. However, for the purposes of applying the >> > >> LCMAP_BYTEREV transformation it seems to be treating the output size >> > >> (in bytes) as character count. So in the example I give, where the >> > >> output sort key is 7 bytes (including the null terminator), it swaps >> > >> *14* bytes--the bytes including the sort key as well as the next 7 >> > >> adjacent bytes. This is obviously a problem if the destination buffer >> > >> is allocated out of some larger memory pool. >> > >> >> > >> This definitely has to be a bug, right? Or at least very poorly >> > >> documented on MS's part. A workaround would either be to not use >> > >> LCMAP_BYTEREV and just swap the bytes manually, or in a second call to >> > >> LCMapStringW with LCMAP_BYTEREV and the correct character count... >> > >> [...] >> Incredible piece of detective work. Yeah, this looks very much like a >> bug in LCMapStringW, embarrassingly one I didn't notice at the time of >> writing the code. I just tested this on W7 and the problem was already >> present. >> >> I don't know if this could be fixed by documentation. There's just no >> safe way to combine LCMAP_SORTKEY with LCMAP_BYTEREV it seems, unless >> you always allocate a buffer at least twice as big as actually required >> for the sort key. >> >> So, yeah, I think manual swaping after calling LCMapStringW without >> LCMAP_BYTEREV is the way to go. I'll prepare a fix. >> >> >> Thanks for tracking down and the testcase, >> Corinna > > Latest snapshot from https://cygwin.com/snapshots/ has the patch applied. > Please try.
I had a look at the fix and it looks good to me; thanks! Erik -- Problem reports: http://cygwin.com/problems.html FAQ: http://cygwin.com/faq/ Documentation: http://cygwin.com/docs.html Unsubscribe info: http://cygwin.com/ml/#unsubscribe-simple
