On Wed, 15 Mar 2006, Daniel Stenberg wrote:
>
>(still haven't reviewed the patch, mostly responding to the questions here)

Do you want a new patch with the changes below (once decisions are made)
instead of reviewing the old patch?

>We can at least assume or pretend that it will and can be used for other
>translations than just EBCDIC <=> ASCII so a more generic name could be
>appropriate I think.

I'm not attached to any of the name change suggestions below, so feel free
to suggest others.

The old CURL_ON_EBCDIC_PLATFORM #define could become either
CURL_ON_NONASCII_PLATFORM or the even more generic CURL_DOES_CONVERSIONS.

The old Curl_convert_ebcdic_to_ascii could become either
Curl_convert_outbound or Curl_convert_to_network_encoding.

The old Curl_convert_ascii_to_ebcdic could become either
Curl_convert_inbound or Curl_convert_from_network_encoding.

The old Curl_convert_utf8_to_ebcdic could become either
Curl_convert_from_utf8_to_host_encoding or the vague Curl_convert_from_utf8.

set.convasciitoebcdic could become set.convfromnetworkencoding
set.convebcdictoascii could become set.convtonetworkencoding
set.convutf8toebcdic could become set.convfromutf8 or
set.convfromutf8tohostencoding
CURLE_EBCDIC_CONV_FAILED could become CURLE_CONV_FAILED
CURLE_EBCDIC_CONV_REQD could become CURLE_CONV_REQD

CONV_ASCII_TO_EBCDIC_FUNCTION, CONV_EBCDIC_TO_ASCII_FUNCTION, and
CONV_UTF8_TO_EBCDIC_FUNCTION would mimic whatever is decided for
Curl_convert_ascii_to_ebcdic, etc.

CURL_USE_ICONV_ON_EBCDIC_PLATFORMS could become CURL_USE_ICONV

And curl_ebcdic_callback could be curl_conv_callback

I think that's all of the EBCDIC specific variable, #define, and flag names.

>For FTP ASCII transfers the current behavour is wrong and documented as
>wrong, so any user who would be surprised by a correction are depending on
>something they shouldn't. And besides, they could easily switch to binary
>mode and get the old functionalty brought back.

OK. I'll remove the #ifdef's so everyone goes through this code...
with the understanding that my capability for testing on non-TPF platforms
is limited.
(That's another way to say it scares me a bit to make major changes for
EVERYONE.)

>>>- There are several places where #ifdefs could be removed from where they
>>>are interspersed within the core curl code and changed to supply
>>>alternate functions depending on the platform, e.g.
>>>curl_convert_lineends() and (a suitably renamed)
>>>Curl_convert_ascii_to_ebcdic() could be no-ops or even macroed away on
>>>non-EBCDIC platforms, making the code easier to read.
>>
>>That's the approach I've used with my Apache contributions, so I'm
>>familiar with it. And I can certainly do that if it's preferred. Sometimes
>>it's a toss up between looking at #ifdefs or tracking down a function to
>>realize that it's been #ifdef'd away.
>
>I'd say it is preferred. Tracking down a function only to find it #ifdef'd
>away is more of a one-time operation, while the whole code flow is easier
>to read when less #ifdef-ridden and that is something that's likely to be
>done many times.

I'm not sure how many #ifdef's are left once we open up the translation and
line end processing for all platforms.
I'll look at that after those changes.

>>>- curl_unescape2() shouldn't take a void *; that pointer must be a
>>>struct SessionHandle *
>>
>>I made it a void pointer for backward compatibility and for use outside of
>>libcurl.
>
>Yes, it should actually more accurately use a 'CURL *' as that what other
>external functions use for this. It struck me that perhaps a better name
>for this new function would be curl_easy_[un]escape() to better match the
>other functions that take a CURL * as first argument?

I'll change these to curl_easy_[un]escape(CURL *handle, const char *string,
int length)

-David McCreedy
Received on 2006-03-15