[
https://issues.apache.org/jira/browse/TS-629?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003028#comment-13003028
]
Nick Kew edited comment on TS-629 at 3/10/11 10:47 AM:
-------------------------------------------------------
Hmm, trying to upload patch, but the Provide Patch button just marks
PatchAvailable without giving me the chance to upload!
Guess I'll paste the patch here.
This patch presumes 8-bit chars, and may therefore raise portability issues
itself. I need to raise that on-list.
{code}
--- old/trafficserver-2.1.5-unstable/iocore/cluster/ClusterHandlerBase.cc
2011-01-07 23:55:18.000000000 +0000
+++ trafficserver-2.1.5-unstable/iocore/cluster/ClusterHandlerBase.cc
2011-03-05 18:26:57.583633356 +0000
@@ -145,11 +145,8 @@
ink_release_assert(*(((uint8_t *) data) - DATA_HDR + 1) == (uint8_t)
ALLOC_DATA_MAGIC);
*(((uint8_t *) data) - DATA_HDR + 1) = (uint8_t) ~ ALLOC_DATA_MAGIC;
- if (size_index >= 0) {
- ink_release_assert(*(((char *) data) - DATA_HDR) == size_index);
- } else {
- ink_release_assert(*(((char *) data) - DATA_HDR) == -1);
- }
+ /* TS-629: remove presumed-unnecessary test */
+ ink_release_assert(*(((char *) data) - DATA_HDR) == size_index);
} else {
// malloc'ed memory, not alloced via real_alloc_data().
// Data will be xfree()'ed when IOBufferBlock is freed
--- old/trafficserver-2.1.5-unstable/iocore/cluster/ClusterProcessor.cc
2011-01-07 23:55:18.000000000 +0000
+++ trafficserver-2.1.5-unstable/iocore/cluster/ClusterProcessor.cc
2011-03-05 18:38:16.067130703 +0000
@@ -206,10 +206,10 @@
ink_release_assert(*((uint8_t *) (d - data_hdr + 1)) == (uint8_t)
ALLOC_DATA_MAGIC);
char size_index = *(d - data_hdr);
- if (size_index >= 0) {
- ink_release_assert((0 <= size_index) && (size_index <=
(DEFAULT_BUFFER_SIZES - 1)));
+ if (!(size_index & 0x80)) {
+ ink_release_assert(size_index <= (DEFAULT_BUFFER_SIZES - 1));
} else {
- ink_release_assert(size_index == -1);
+ ink_release_assert(size_index == 0xff);
}
// Extract 'this' pointer
--- old/trafficserver-2.1.5-unstable/lib/ts/ParseRules.h 2011-01-07
23:55:18.000000000 +0000
+++ trafficserver-2.1.5-unstable/lib/ts/ParseRules.h 2011-03-05
18:43:59.015137741 +0000
@@ -189,8 +189,7 @@
#ifndef COMPILE_PARSE_RULES
return (parseRulesCType[(unsigned char) c] & is_char_BIT);
#else
- /* if I don't cast, gnu gives a warning: condition is always true */
- return (c >= 0 && (unsigned char) c <= (unsigned char) 127);
+ return (c&0x80 == 0);
#endif
}
@@ -250,7 +249,7 @@
#ifndef COMPILE_PARSE_RULES
return (parseRulesCType[(unsigned char) c] & is_ctl_BIT);
#else
- return ((c >= 0 && c <= 31) || c == 127);
+ return ((!(c & 0x80) && c <= 31) || c == 127);
#endif
}
--- old/trafficserver-2.1.5-unstable/proxy/mgmt2/cluster/ClusterCom.cc
2011-01-07 23:55:18.000000000 +0000
+++ trafficserver-2.1.5-unstable/proxy/mgmt2/cluster/ClusterCom.cc
2011-01-09 13:50:57.877142034 +0000
@@ -2131,7 +2131,7 @@
memset(tmp_reply, 0, 1024);
while ((res = read_socket(fd, tmp_reply, 1022) > 0)) {
- if (tmp_reply[0] == EOF) {
+ if (tmp_reply[0] == (char)EOF) {
break;
}
reply->copyFrom(tmp_reply, strlen(tmp_reply));
--- old/trafficserver-2.1.5-unstable/proxy/mgmt2/preparse/IPRange.cc
2011-01-07 23:55:18.000000000 +0000
+++ trafficserver-2.1.5-unstable/proxy/mgmt2/preparse/IPRange.cc
2011-01-09 13:56:44.285146794 +0000
@@ -109,7 +109,7 @@
c = EOF; // mimic behavior of getc on EOF
// append null
line[n] = '\0';
- if (c == EOF)
+ if (c == (char)EOF)
end_of_file = true;
// first_string has the same length as line, so here we disable coverity
check
// coverity[secure_coding]
{code}
was (Author: n...@webthing.com):
Hmm, trying to upload patch, but the Provide Patch button just marks
PatchAvailable without giving me the chance to upload!
Guess I'll paste the patch here.
This patch presumes 8-bit chars, and may therefore raise portability issues
itself. I need to raise that on-list.
--- old/trafficserver-2.1.5-unstable/iocore/cluster/ClusterHandlerBase.cc
2011-01-07 23:55:18.000000000 +0000
+++ trafficserver-2.1.5-unstable/iocore/cluster/ClusterHandlerBase.cc
2011-03-05 18:26:57.583633356 +0000
@@ -145,11 +145,8 @@
ink_release_assert(*(((uint8_t *) data) - DATA_HDR + 1) == (uint8_t)
ALLOC_DATA_MAGIC);
*(((uint8_t *) data) - DATA_HDR + 1) = (uint8_t) ~ ALLOC_DATA_MAGIC;
- if (size_index >= 0) {
- ink_release_assert(*(((char *) data) - DATA_HDR) == size_index);
- } else {
- ink_release_assert(*(((char *) data) - DATA_HDR) == -1);
- }
+ /* TS-629: remove presumed-unnecessary test */
+ ink_release_assert(*(((char *) data) - DATA_HDR) == size_index);
} else {
// malloc'ed memory, not alloced via real_alloc_data().
// Data will be xfree()'ed when IOBufferBlock is freed
--- old/trafficserver-2.1.5-unstable/iocore/cluster/ClusterProcessor.cc
2011-01-07 23:55:18.000000000 +0000
+++ trafficserver-2.1.5-unstable/iocore/cluster/ClusterProcessor.cc
2011-03-05 18:38:16.067130703 +0000
@@ -206,10 +206,10 @@
ink_release_assert(*((uint8_t *) (d - data_hdr + 1)) == (uint8_t)
ALLOC_DATA_MAGIC);
char size_index = *(d - data_hdr);
- if (size_index >= 0) {
- ink_release_assert((0 <= size_index) && (size_index <=
(DEFAULT_BUFFER_SIZES - 1)));
+ if (!(size_index & 0x80)) {
+ ink_release_assert(size_index <= (DEFAULT_BUFFER_SIZES - 1));
} else {
- ink_release_assert(size_index == -1);
+ ink_release_assert(size_index == 0xff);
}
// Extract 'this' pointer
--- old/trafficserver-2.1.5-unstable/lib/ts/ParseRules.h 2011-01-07
23:55:18.000000000 +0000
+++ trafficserver-2.1.5-unstable/lib/ts/ParseRules.h 2011-03-05
18:43:59.015137741 +0000
@@ -189,8 +189,7 @@
#ifndef COMPILE_PARSE_RULES
return (parseRulesCType[(unsigned char) c] & is_char_BIT);
#else
- /* if I don't cast, gnu gives a warning: condition is always true */
- return (c >= 0 && (unsigned char) c <= (unsigned char) 127);
+ return (c&0x80 == 0);
#endif
}
@@ -250,7 +249,7 @@
#ifndef COMPILE_PARSE_RULES
return (parseRulesCType[(unsigned char) c] & is_ctl_BIT);
#else
- return ((c >= 0 && c <= 31) || c == 127);
+ return ((!(c & 0x80) && c <= 31) || c == 127);
#endif
}
--- old/trafficserver-2.1.5-unstable/proxy/mgmt2/cluster/ClusterCom.cc
2011-01-07 23:55:18.000000000 +0000
+++ trafficserver-2.1.5-unstable/proxy/mgmt2/cluster/ClusterCom.cc
2011-01-09 13:50:57.877142034 +0000
@@ -2131,7 +2131,7 @@
memset(tmp_reply, 0, 1024);
while ((res = read_socket(fd, tmp_reply, 1022) > 0)) {
- if (tmp_reply[0] == EOF) {
+ if (tmp_reply[0] == (char)EOF) {
break;
}
reply->copyFrom(tmp_reply, strlen(tmp_reply));
--- old/trafficserver-2.1.5-unstable/proxy/mgmt2/preparse/IPRange.cc
2011-01-07 23:55:18.000000000 +0000
+++ trafficserver-2.1.5-unstable/proxy/mgmt2/preparse/IPRange.cc
2011-01-09 13:56:44.285146794 +0000
@@ -109,7 +109,7 @@
c = EOF; // mimic behavior of getc on EOF
// append null
line[n] = '\0';
- if (c == EOF)
+ if (c == (char)EOF)
end_of_file = true;
// first_string has the same length as line, so here we disable coverity
check
// coverity[secure_coding]
> non-portable char vs integer comparisons
> ----------------------------------------
>
> Key: TS-629
> URL: https://issues.apache.org/jira/browse/TS-629
> Project: Traffic Server
> Issue Type: Bug
> Components: Portability
> Affects Versions: 2.1.5
> Environment: ARM/Maemo with gcc 4.2.1
> Reporter: Nick Kew
> Priority: Minor
> Fix For: 2.1.7
>
>
> Several char comparisons fail with "always true" or "always false" on this
> platform:
> iocore/cluster/ClusterHandlerBase.cc line 148 (is this needed? Under what
> circumstances can size_index be negative but not -1)
> iocore/cluster/ClusterProcessor.cc lines 209/210/212 (as above?)
> lib/ts/ParseRules.h line 193 and 253 (platform is fine if we test bitwise-AND
> 0x80 for negativity)
> proxy/mgmt2/cluster/ClusterCom.cc line 2134 (casting EOF to char fixes it)
> proxy/mgmt2/preparse/IPRange.cc line 112 (as above)
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
