[SAMBA3] Really confused about convert_string returns
Jeremy Allison
jra at samba.org
Wed Apr 1 00:19:03 GMT 2009
On Wed, Apr 01, 2009 at 10:22:37AM +1100, Andrew Bartlett wrote:
> While working on my failed attempt to merge the charcnv core (I still
> propose to merge the APIs), I was following the convert_string and
> convert_string_internal APIs.
>
> The use of the return value in this code is rather inconsistent, and I'm
> a little confused:
>
> Firstly, it does not seem to return -1 in the error cases (see patch).
> Even if it did, then in some other cases it has:
>
> return retval + convert_string_internal(...
>
> where retval can be an integer representing how many bytes have been
> converted in the the destination charset so far, but
> convert_string_internal() can return (size_t)-1!
>
> Is it presumed that retval + (size_t)-1 will always be -1?
>
> Anyway, I wondered if someone might like to take a look a this.
Thanks ! I think this is the correct fix (E2BIG shouldn't
return (size_t)-1 as it's not a fatal error).
Testing and will commit if it passes.
Cheers,
Jeremy.
-------------- next part --------------
diff --git a/source3/lib/charcnv.c b/source3/lib/charcnv.c
index c3b3451..03b32c1 100644
--- a/source3/lib/charcnv.c
+++ b/source3/lib/charcnv.c
@@ -242,7 +242,7 @@ static size_t convert_string_internal(charset_t from, charset_t to,
DEBUG(3,("convert_string_internal: Conversion error: %s(%s)\n",reason,inbuf));
if (allow_bad_conv)
goto use_as_is;
- break;
+ return (size_t)-1;
case E2BIG:
reason="No more room";
if (!conv_silent) {
@@ -263,11 +263,12 @@ static size_t convert_string_internal(charset_t from, charset_t to,
DEBUG(3,("convert_string_internal: Conversion error: %s(%s)\n",reason,inbuf));
if (allow_bad_conv)
goto use_as_is;
- break;
+
+ return (size_t)-1;
default:
if (!conv_silent)
DEBUG(0,("convert_string_internal: Conversion error: %s(%s)\n",reason,inbuf));
- break;
+ return (size_t)-1;
}
/* smb_panic(reason); */
}
@@ -412,7 +413,11 @@ size_t convert_string(charset_t from, charset_t to,
#ifdef BROKEN_UNICODE_COMPOSE_CHARACTERS
goto general_case;
#else
- return retval + convert_string_internal(from, to, p, slen, q, dlen, allow_bad_conv);
+ size_t ret = convert_string_internal(from, to, p, slen, q, dlen, allow_bad_conv);
+ if (ret == (size_t)-1) {
+ return ret;
+ }
+ return retval + ret;
#endif
}
}
@@ -448,7 +453,11 @@ size_t convert_string(charset_t from, charset_t to,
#ifdef BROKEN_UNICODE_COMPOSE_CHARACTERS
goto general_case;
#else
- return retval + convert_string_internal(from, to, p, slen, q, dlen, allow_bad_conv);
+ size_t ret = convert_string_internal(from, to, p, slen, q, dlen, allow_bad_conv);
+ if (ret == (size_t)-1) {
+ return ret;
+ }
+ return retval + ret;
#endif
}
}
@@ -484,7 +493,11 @@ size_t convert_string(charset_t from, charset_t to,
#ifdef BROKEN_UNICODE_COMPOSE_CHARACTERS
goto general_case;
#else
- return retval + convert_string_internal(from, to, p, slen, q, dlen, allow_bad_conv);
+ size_t ret = convert_string_internal(from, to, p, slen, q, dlen, allow_bad_conv);
+ if (ret == (size_t)-1) {
+ return ret;
+ }
+ return retval + ret;
#endif
}
}
More information about the samba-technical
mailing list