meebey / smuxi

Smuxi is an user-friendly and free IRC client for Linux, Windows and Mac OS X based on GNOME / GTK+
https://smuxi.im/
GNU General Public License v2.0
172 stars 46 forks source link

UTF-32-related fixes to Frontend-STFL #235

Closed RavuAlHemio closed 9 years ago

RavuAlHemio commented 9 years ago

Remove code duplication, fix serialization for big-endian systems, work around Mono.Unix.UnixMarshal.PtrToString breakage with UTF-32, fix memory leaks.

knocte commented 9 years ago

3 nits:

RavuAlHemio commented 9 years ago

Thanks for your feedback; I have updated the pull request.

knocte commented 9 years ago

2nd commit still doesn't state the why, does it crash if the patch is not committed?

WRT 3rd commit, I would change the signature to not use "out" parameters, rename it to IsMonoVersionOlderThan(), and simply pass (4, 2) as parameters. It would simply return false also if running under MS.NET. The way you have implemented it now, the check would return false for mono 3.x.

RavuAlHemio commented 9 years ago

I think the bug fixed by the 3rd commit was introduced with Mono moving to the ReferenceSource implementations of many classes, including UTF32Encoding. Therefore, I think it only hit some versions within 4.0.0 <= mono < 4.2.0. However, it probably makes sense to be cautious anyway.

meebey commented 9 years ago

From Travis-CII:

./STFL/StflApi.cs(118,23): error CS0246: The type or namespace name `StringOnHeap' could not be found. Are you missing a using directive or an assembly reference?

STFL/StflApi.cs needs to be added to the Makefile.am

meebey commented 9 years ago

The 3rd commit should state why workaround is needed in the commit long message, mostly your comment would be sufficient in there with a link to the upstream bugreport