You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@tomcat.apache.org by Ken Cheung <ms...@gmail.com> on 2012/02/28 17:40:17 UTC

Inconsistency in AjpMessage.java

I observed some code clones in Tomcat and found inconsistent code. Could anyone explain why this is not a bug?

/tomcat/trunk/java/org/apache/coyote/ajp/AjpMessage.java

195            if (cc == null) {
196                log.error(sm.getString("ajpmessage.null"),
197                        new NullPointerException());
198                appendInt(0);
199                appendByte(0);
200                return;
201            }
202            int start = cc.getStart();
203            int end = cc.getEnd();
204            appendInt(end - start);
205            char[] cbuf = cc.getBuffer();
206            for (int i = start; i < end; i++) {
207                char c = cbuf[i];
208                // Note:  This is clearly incorrect for many strings,
209                // but is the only consistent approach within the current
210                // servlet framework.  It must suffice until servlet output
211                // streams properly encode their output.
212                if (((c <= 31) && (c != 9)) || c == 127 || c > 255) {
213                    c = ' ';
214                }
215                appendByte(c);
216            }
217            appendByte(0);

/tomcat/trunk/java/org/apache/coyote/ajp/AjpMessage.java

230            if (str == null) {
231                log.error(sm.getString("ajpmessage.null"),
232                        new NullPointerException());
233                appendInt(0);
234                appendByte(0);
235                return;
236            }
237            int len = str.length();
238            appendInt(len);
239            for (int i = 0; i < len; i++) {
240                char c = str.charAt (i);
241                // Note:  This is clearly incorrect for many strings,
242                // but is the only consistent approach within the current
243                // servlet framework.  It must suffice until servlet output
244                // streams properly encode their output.
245                if (((c <= 31) && (c != 9)) || c == 127 || c > 255) {
246                    c = ' ';
247                }
248                appendByte(c);
249            }
250            appendByte(0);

Quick description of the inconsistency
Two code snippets are very similar code, but as you see, both codes use different ways to compute the condition of for loop.


Re: Inconsistency in AjpMessage.java

Posted by Tim Watts <ti...@cliftonfarm.org>.
On Wed, 2012-02-29 at 00:40 +0800, Ken Cheung wrote:
> I observed some code clones in Tomcat and found inconsistent code.
> Could anyone explain why this is not a bug?
> 
If you think it's a bug can you explain why you think so?  You may get
more value out of the homework assignment by pondering this.

> /tomcat/trunk/java/org/apache/coyote/ajp/AjpMessage.java
> 
> 195            if (cc == null) {
> 196                log.error(sm.getString("ajpmessage.null"),
> 197                        new NullPointerException());
> 198                appendInt(0);
> 199                appendByte(0);
> 200                return;
> 201            }
> 202            int start = cc.getStart();
> 203            int end = cc.getEnd();
> 204            appendInt(end - start);
> 205            char[] cbuf = cc.getBuffer();
> 206            for (int i = start; i < end; i++) {
> 207                char c = cbuf[i];
> 208                // Note:  This is clearly incorrect for many strings,
> 209                // but is the only consistent approach within the current
> 210                // servlet framework.  It must suffice until servlet output
> 211                // streams properly encode their output.
> 212                if (((c <= 31) && (c != 9)) || c == 127 || c > 255) {
> 213                    c = ' ';
> 214                }
> 215                appendByte(c);
> 216            }
> 217            appendByte(0);
> 
> /tomcat/trunk/java/org/apache/coyote/ajp/AjpMessage.java
> 
> 230            if (str == null) {
> 231                log.error(sm.getString("ajpmessage.null"),
> 232                        new NullPointerException());
> 233                appendInt(0);
> 234                appendByte(0);
> 235                return;
> 236            }
> 237            int len = str.length();
> 238            appendInt(len);
> 239            for (int i = 0; i < len; i++) {
> 240                char c = str.charAt (i);
> 241                // Note:  This is clearly incorrect for many strings,
> 242                // but is the only consistent approach within the current
> 243                // servlet framework.  It must suffice until servlet output
> 244                // streams properly encode their output.
> 245                if (((c <= 31) && (c != 9)) || c == 127 || c > 255) {
> 246                    c = ' ';
> 247                }
> 248                appendByte(c);
> 249            }
> 250            appendByte(0);
> 
> Quick description of the inconsistency
> Two code snippets are very similar code, but as you see, both codes
> use different ways to compute the condition of for loop.
>