Data conversions: strings #10
Conversation
- add the missing conversions from string to permitted data types
driver/connect.c
Outdated
| @@ -1002,7 +1002,7 @@ static SQLRETURN process_config(esodbc_dbc_st *dbc, config_attrs_st *attrs) | |||
| /* | |||
| * request timeout for liburl: negative reset to 0 | |||
| */ | |||
| if (! wstr2long(&attrs->timeout, &timeout)) { | |||
| if (! wstr2llong(&attrs->timeout, &timeout)) { | |||
| ERRH(dbc, "failed to convert '" LWPDL "' to long.", | |||
There was a problem hiding this comment.
The error message could be updated from long to long long as well.
Same on lines 1021 and 1038.
driver/util.c
Outdated
| } | ||
|
|
||
| for ( ; i < val->cnt; i ++) { | ||
| for (res = 0; i < val->cnt; i ++) { | ||
| /* is it a number? */ | ||
| if (val->str[i] < L'0' || L'9' < val->str[i]) { | ||
| return FALSE; |
There was a problem hiding this comment.
I think this should set errno before returning. In one place that calls this you are assuming that errno will be zero for this return case, but it might be something else left over from a previous error if you don't explicitly set it to zero. So either set it to 0 before returning here and leave the calling code as-is, or set it to EINVAL (which matches what strtoull() would do on most operating systems other than Windows) and change the calling code too.
Same on line 79.
There was a problem hiding this comment.
Fixed by adding EINVAL. This is indeed the better solution: if one failure uses errno, the others should too.
I couldn't find the code location assuming a zero value in errno, without it being cleared beforehand (thought that might be "normal", if I did the mistake before too :-) ). But I hope it's then fixed by introducing EINVAL.
Thanks!
There was a problem hiding this comment.
I couldn't find the code location assuming a zero value in
errno
Sorry, I should have been clearer. It was this, around line 1861:
RET_HDIAGS(stmt, errno ? SQL_STATE_22003 : SQL_STATE_22018)
That would return SQLSTATE 22003 for any non-zero value of errno, thus losing the distinction between syntax error and out-of-range.
However, it's all fixed now as you've changed errno to errno == ERANGE on that line, so no further changes required.
driver/queries.c
Outdated
| errno = 0; | ||
| if (! wstr2llong(&wval, &ll)) { /* string is 0-term -> wcstoll? */ | ||
| ERRH(stmt, "can't convert `" LWPD "` to long long.", wstr); | ||
| RET_HDIAGS(stmt, errno ? SQL_STATE_22003 : SQL_STATE_22018); |
There was a problem hiding this comment.
If you were to change wstr2llong() and wstr2ullong() to set errno to EINVAL for invalid input then errno would need changing to errno == ERANGE here.
driver/queries.c
Outdated
| /* if empty string, non-numeric or under/over-flow, bail out */ | ||
| if ((! wval.cnt) || (wval.str + wval.cnt != endp) || errno) { | ||
| ERRH(stmt, "can't convert `" LWPD "` to double.", wstr); | ||
| RET_HDIAGS(stmt, errno ? SQL_STATE_22003 : SQL_STATE_22018); |
There was a problem hiding this comment.
It would be more portable to replace errno with errno == ERANGE here, as strtod() could set errno to EINVAL on some platforms (not Windows I agree).
There was a problem hiding this comment.
True. Fixed, thanks.
driver/queries.c
Outdated
| default: | ||
| // FIXME: convert data | ||
| FIXME; | ||
| BUGH(stmt, "unexpected unhanlded data type: %d.", |
There was a problem hiding this comment.
typo: unhanlded -> unhandled
- have wstr conversions to longs also return EINVAL on failed conversion; - adapt error propagation to this change; - adjust error reporting to correctly log failed conversion.
This PR is the last part of data conversions and adds the remainder of the conversions from string to various SQL C data types.
The new conversions reuse the existing conversion code so PR is rather small: convert first the string to
doubleorlong long(unsigned long longis separate case though), then just use the numeric conversions.