Bug 3819

Summary: libxkbfile open codes strcasecmp
Product: xorg Reporter: Adam Jackson <ajax>
Component: Lib/libxkb*Assignee: Daniel Stone <daniel>
Status: RESOLVED FIXED QA Contact: Xorg Project Team <xorg-team>
Severity: normal    
Priority: high CC: bernie, Don
Version: 6.8.99.16Keywords: janitor
Hardware: All   
OS: All   
Whiteboard: 2011BRB_Reviewed
i915 platform: i915 features:
Bug Depends on:    
Bug Blocks: 6141    
Attachments:
Description Flags
Fix none

Description Adam Jackson 2005-07-19 23:35:53 UTC
... which is totally already part of posix.  but in this case it allocates
fixed-size buffers on the stack, so it'll false-match on strings over 512 bytes
long.  hooray!
Comment 1 Joe Luquette 2006-02-03 07:07:07 UTC
(In reply to comment #0)
> ... which is totally already part of posix.  but in this case it allocates
> fixed-size buffers on the stack, so it'll false-match on strings over 512 bytes
> long.  hooray!

So is the desired fix to rework strcasecmp to not allocated fixed size buffers,
or would we rather remove it and require a strcasecmp be present in libc?
Comment 2 Adam Jackson 2006-02-03 08:00:24 UTC
(In reply to comment #1)
> So is the desired fix to rework strcasecmp to not allocated fixed size buffers,
> or would we rather remove it and require a strcasecmp be present in libc?

both:

- use the operating system's strcasecmp where present
- use a non-broken version (probably from BSD libc) where not present
Comment 3 Daniel Stone 2006-03-05 20:59:24 UTC
where?
Comment 4 Daniel Stone 2006-03-05 20:59:40 UTC
hot comment action
Comment 5 Daniel Stone 2006-03-05 20:59:54 UTC
more comments
Comment 6 Don Scorgie 2006-03-21 06:36:19 UTC
Created attachment 5003 [details] [review]
Fix

The patch checks for strcasecmp in configure.ac and if it isn't there, defines
NEED_STRCASECMP.

In xkbmisc.c, if this is defined, the casecmp function (_XkbStrCaseCmp) is
replaced with a new version (from the freeBSD implementation).	If strcasecmp
is already present, this is #define'd to strcasecmp.

Is this the right way to do it?  Or would it be better to change all instances
of _XkbStrCaseCmp to strcasecmp and do it that way?
Comment 7 Daniel Stone 2006-03-26 09:15:41 UTC
even better, just removed all use of _XkbStrCaseCmp
Comment 8 Bernie Innocenti 2006-03-27 09:30:43 UTC
This patch appears to be bogus.  tolower() works with
single characters, not strings:

xkbtext.c:1428: warning: passing argument 1 of 'tolower' makes integer from
pointer without a cast
xkbtext.c:1428: warning: cast from pointer to integer of different size
xkbtext.c:1428: warning: cast to pointer from integer of different size
xkbtext.c:1428: warning: initialization makes integer from pointer without a cast

Comment 9 Adam Jackson 2006-03-27 14:23:29 UTC
that patch wasn't applied, so, this is still closed.
Comment 10 Bernie Innocenti 2006-03-27 15:17:13 UTC
I wasn't referring to the patch attached to this bug.
The problem is in the patch that went to lib/xkbfile
two days ago:

 revision 1.7
 date: 2006-03-25 23:15:58 +0000;  author: daniels;  state: Exp;  lines: +5 -5
 Bug #3819: Change open-coded _XkbStrCaseCmp to strcmp + tolower.

The new code passes a pointer argument to tolower() in
several places:

 1.7          (daniels  25-Mar-06):          else if
(strcmp(tolower(tok),"layout") == 0)
 1.1          (kaleb    14-Nov-03):              headingtype = HEAD_LAYOUT;
 1.7          (daniels  25-Mar-06):          else if
(strcmp(tolower(tok),"variant") == 0)
 1.1          (kaleb    14-Nov-03):              headingtype = HEAD_VARIANT;
 1.7          (daniels  25-Mar-06):          else if
(strcmp(tolower(tok),"option") == 0)


It also breaks building xserver because there are still
references to _XkbStrCaseCmp() in the xorg tree.
Comment 11 Daniel Stone 2006-03-28 07:11:10 UTC
unbroken in HEAD
Comment 12 Bernie Innocenti 2006-03-29 04:31:55 UTC
Fixed as of yesterday, thanks.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.