Re-introduce private implementations of mbstowcs/wcstombs for wxQt-Android

[Mariano Reingart]

Dear

Sean D'Epagnier & David Register sent a pull request with several
fixes to get OpenCPN working under Android (wxQt)

One of that changes re-introduces wxWcstombs and wxMbstowcs own
wxWidgets custom implementation:

https://github.com/seandepagnier/wxWidgets/commit/9f2e1b16d125a6c5adb22b72ca7213f0e71feab9

These are needed due missing/broken android support, but they where
removed in r75024 by VZ:

> Always use standard mbstowcs() and wcstombs() functions.
>
> Don't provide our own (not fully functional) definitions of them and always
> use the system versions as we don't support OS X 10.2 which was the last
> platform where these functions didn't exist/work.

http://trac.wxwidgets.org/ticket/15580

The question is:

Is there any objection to re-introduce the replacements?
(they may be too simple and error prone in some situations, but they
will help wxQt Android to advance until a better implementation is
found)

As they were removed, I don't think they will cause any harm to the other ports.

Best regards,

Mariano Reingart
http://www.sistemasagiles.com.ar
http://reingart.blogspot.com

[Vadim Zeitlin]

On Sun, 18 Jan 2015 21:57:32 -0300 Mariano Reingart wrote:

MR> One of that changes re-introduces wxWcstombs and wxMbstowcs own
MR> wxWidgets custom implementation:
MR>
MR> https://github.com/seandepagnier/wxWidgets/commit/9f2e1b16d125a6c5adb22b72ca7213f0e71feab9
MR>
MR> These are needed due missing/broken android support, but they where
MR> removed in r75024 by VZ:
MR>
MR> > Always use standard mbstowcs() and wcstombs() functions.
MR> >
MR> > Don't provide our own (not fully functional) definitions of them and always
MR> > use the system versions as we don't support OS X 10.2 which was the last
MR> > platform where these functions didn't exist/work.

The trouble with these functions is that they not just "not fully
functional", I was uncharacteristically polite in that commit message: they
are just completely broken and basically produce complete garbage with
non-ASCII characters.

MR> Is there any objection to re-introduce the replacements?
MR> (they may be too simple and error prone in some situations, but they
MR> will help wxQt Android to advance until a better implementation is
MR> found)

I can't in good conscience object to their use without providing better
implementations and I don't know how difficult this would be (I'm sure it
should be pretty trivial for Qt which must have the required functionality
built in, but don't know about "raw" Android). But I'd really like to
somehow ensure that these functions are not accidentally used in anything
that is actually supposed to work. A warning during configure time is
almost certainly not enough to guarantee it, so we probably need a #warning
in the sources as well. Ideal, from my point of view, would be to make
these functions private to wxAndroid or wxQt to at least make certain they
are never accidentally used in the other ports.

Thanks,
VZ

[Mariano Reingart]

Ok, I'll see what can I do (I'll retry to contact Sean again)

What about this change: "for android, add local definitions of wcstol
wcstoul and wcstod"
> These functions are needed by wxString::ToLong wxString::ToDouble etc..
> They do exist in the android library, but do not work correctly.
> This change implements them using strtol strtoul and strtod

https://github.com/seandepagnier/wxWidgets/commit/de7f4ed4c9519df65a468921c83b5d6641173c34

I didn't have time to see it in depth nor test it
Also, I've still to compile wxQT for Android, but I'm confident Sean
tested this with OpenCPN

BTW, I've commited a Sean's changes that also could affect
"standalone" Android, is that ok too?
(I think it is harmless, see r78379)

[Vadim Zeitlin]

On Mon, 19 Jan 2015 02:00:00 -0300 Mariano Reingart wrote:

MR> What about this change: "for android, add local definitions of wcstol
MR> wcstoul and wcstod"
MR> > These functions are needed by wxString::ToLong wxString::ToDouble etc..
MR> > They do exist in the android library, but do not work correctly.
MR> > This change implements them using strtol strtoul and strtod
MR>
MR> https://github.com/seandepagnier/wxWidgets/commit/de7f4ed4c9519df65a468921c83b5d6641173c34

This is better: first, we don't care about non-ASCII characters when
parsing numbers anyhow (although I'm not sure if these functions currently
correctly fail in this case) and, second, it's clearly Android-specific, so
I don't have any major problems with this.

MR> BTW, I've commited a Sean's changes that also could affect
MR> "standalone" Android, is that ok too?
MR> (I think it is harmless, see r78379)

It's ok, but I agree that it would be better to fix the check in configure
instead of working around wrong detection results in the code.

Thanks,
VZ