You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by rb...@covalent.net on 2000/07/03 04:50:54 UTC
util_xml.c has constification warnings.
I just compiled the most recent code, and util_xml.c has some const
warnings from using strchr. In investigating this, I discovered that
ap_strchr_c is defined incorrectly. I am assuming that ap_strchr_c is
meant to conform to the Single Unix Spec exactly. We are defining it as
const char * ap_strchr_c(const char *, int)
Single Unix defines it as
char * strchr(const char *, int)
By changing our definition appropriately, and calling ap_strchr_c as in
the patch below, we remove all warnings from the server. Does anybody
mind if I commit this patch?
Ryan
? conf.out
? build.log
? build.err
Index: include/httpd.h
===================================================================
RCS file: /home/cvs/apache-2.0/src/include/httpd.h,v
retrieving revision 1.64
diff -u -d -b -w -u -r1.64 httpd.h
--- include/httpd.h 2000/06/30 21:18:13 1.64
+++ include/httpd.h 2000/07/03 02:47:34
@@ -1038,9 +1038,9 @@
# define strstr(s, c) ap_strstr(s,c)
char *ap_strchr(char *s, int c);
-const char *ap_strchr_c(const char *s, int c);
+char *ap_strchr_c(const char *s, int c);
char *ap_strrchr(char *s, int c);
-const char *ap_strrchr_c(const char *s, int c);
+char *ap_strrchr_c(const char *s, int c);
char *ap_strstr(char *s, char *c);
const char *ap_strstr_c(const char *s, const char *c);
Index: main/util_debug.c
===================================================================
RCS file: /home/cvs/apache-2.0/src/main/util_debug.c,v
retrieving revision 1.2
diff -u -d -b -w -u -r1.2 util_debug.c
--- main/util_debug.c 2000/06/24 19:31:41 1.2
+++ main/util_debug.c 2000/07/03 02:47:47
@@ -64,7 +64,7 @@
char *ap_strchr(char *s, int c)
{ return strchr(s,c); }
-const char *ap_strchr_c(const char *s, int c)
+char *ap_strchr_c(const char *s, int c)
{ return strchr(s,c); }
# undef strrchr
@@ -72,7 +72,7 @@
char *ap_strrchr(char *s, int c)
{ return strrchr(s,c); }
-const char *ap_strrchr_c(const char *s, int c)
+char *ap_strrchr_c(const char *s, int c)
{ return strrchr(s,c); }
#undef strstr
Index: main/util_xml.c
===================================================================
RCS file: /home/cvs/apache-2.0/src/main/util_xml.c,v
retrieving revision 1.4
diff -u -d -b -w -u -r1.4 util_xml.c
--- main/util_xml.c 2000/07/01 14:02:49 1.4
+++ main/util_xml.c 2000/07/03 02:47:47
@@ -258,7 +258,7 @@
elem->lang = elem->parent->lang;
/* adjust the element's namespace */
- colon = strchr(elem->name, ':');
+ colon = ap_strchr_c(elem->name, ':');
if (colon == NULL) {
/*
* The element is using the default namespace, which will always
@@ -283,7 +283,7 @@
/* adjust all remaining attributes' namespaces */
for (attr = elem->attr; attr; attr = attr->next) {
- colon = strchr(attr->name, ':');
+ colon = ap_strchr_c(attr->name, ':');
if (colon == NULL) {
/*
* Attributes do NOT use the default namespace. Therefore,
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: util_xml.c has constification warnings.ry
Posted by Dirk-Willem van Gulik <di...@covalent.net>.
On Mon, 3 Jul 2000, Greg Stein wrote:
> On Mon, Jul 03, 2000 at 11:31:14AM +0200, Sascha Schumann wrote:
> >... re: ap_strchr and ap_strchr_c
>
> > Did you ever try to get such a function added to the ISO C
> > standard? If it is useful to Apache, it might be useful to other
> > software as well.
Go to www.ieee.org, then send mail to oropose it to Workgroup 14 or was it
12.
strchr_c might be reasonably be easy. But the changed semantics of strchr
with the null might be to had a sell.
> Regardless: yes, it would be quite useful to others. Getting it there is a
> different matter...
Actually, having sat on such boards, it is this sort of things which
really drives it (and the right timing of course).
Dw
Re: util_xml.c has constification warnings.ry
Posted by Sascha Schumann <sa...@schumann.cx>.
> Well, the easy part is to get into workgroup 14,
Clarification: 'Easy' refers here to have just enough $$$. I
don't think there are any technical reasons why an organization
could not join the respective ISO committee.
- Sascha
Re: util_xml.c has constification warnings.ry
Posted by Sascha Schumann <sa...@schumann.cx>.
On Mon, 3 Jul 2000, Greg Stein wrote:
> On Mon, Jul 03, 2000 at 11:31:14AM +0200, Sascha Schumann wrote:
> >... re: ap_strchr and ap_strchr_c
>
> > Did you ever try to get such a function added to the ISO C
> > standard? If it is useful to Apache, it might be useful to other
> > software as well.
>
> *rofl*
>
> Add it to the ISO standard? Oh geez. I'm dying here... :-)
>
> I wouldn't even know where to start. And besides, it was Ben's idea rather
> than mine. I'll give him the credit... (and he can talk to ISO :-)
Well, the easy part is to be to get into workgroup 14, the WG
which is responsible for IEEE 9899 and reviewing C99. The hard
part is to convince the members that there is indeed a need for
adding strchr_c. Chances that strchr will be changed to take a
char * are equal to 0 though.
Now the good news: Once the change has been adopted by the
workgroup, it will semi-automatically propagate to other relevant
standards (like Single Unix Spec IV, if you do it timely).
> Regardless: yes, it would be quite useful to others. Getting it there is a
> different matter...
Sure, but it won't happen automatically. Someone has to start.
- Sascha
Re: util_xml.c has constification warnings.ry
Posted by Greg Stein <gs...@lyra.org>.
On Mon, Jul 03, 2000 at 11:31:14AM +0200, Sascha Schumann wrote:
>... re: ap_strchr and ap_strchr_c
> Did you ever try to get such a function added to the ISO C
> standard? If it is useful to Apache, it might be useful to other
> software as well.
*rofl*
Add it to the ISO standard? Oh geez. I'm dying here... :-)
I wouldn't even know where to start. And besides, it was Ben's idea rather
than mine. I'll give him the credit... (and he can talk to ISO :-)
Regardless: yes, it would be quite useful to others. Getting it there is a
different matter...
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: util_xml.c has constification warnings.ry
Posted by Sascha Schumann <sa...@schumann.cx>.
On Mon, 3 Jul 2000, Greg Stein wrote:
> On Mon, Jul 03, 2000 at 11:03:48AM +0200, Sascha Schumann wrote:
> > On Mon, 3 Jul 2000, Greg Stein wrote:
> >...
> > > The only reason that we have the ap_strchr_c() is because the Single Unix
> > > Spec is broken. It gives back a writable string when you feed it an
> > > *unwriteable* one. In other words, it is dropping the const and it really
> > > shouldn't.
> >
> > Your analysis is implying that you can pass only read-only
> > strings to strchr. This is false.
> >
> > The prototype was written that way to avoid casts. Imagine this:
> >
> > char buf[256];
> > char *p;
> >
> > sprintf(buf, "for");
> > p = strchr(buf, 'f');
> > if (p) *p = 'x';
> >
> > With your proposed change, the fifth line would need to be
> > rewritten as:
> >
> > p = (char *) strchr(buf, 'f');
> >
> > A 'const' declarator in a parameter declaration does not mean
> > that the respective argument must be a read-only object. It means
> > that the function may not alter that argument. That is why
> > ap_strchr_c should return a value of type char *.
>
> Slow down there, Tex. The Single Unix Spec is broken in the sense that it
> loses the const when we don't want it to.
That sounds much better than your earlier statement.
Did you ever try to get such a function added to the ISO C
standard? If it is useful to Apache, it might be useful to other
software as well.
- Sascha
Re: util_xml.c has constification warnings.ry
Posted by Greg Stein <gs...@lyra.org>.
On Mon, Jul 03, 2000 at 11:03:48AM +0200, Sascha Schumann wrote:
> On Mon, 3 Jul 2000, Greg Stein wrote:
>...
> > The only reason that we have the ap_strchr_c() is because the Single Unix
> > Spec is broken. It gives back a writable string when you feed it an
> > *unwriteable* one. In other words, it is dropping the const and it really
> > shouldn't.
>
> Your analysis is implying that you can pass only read-only
> strings to strchr. This is false.
>
> The prototype was written that way to avoid casts. Imagine this:
>
> char buf[256];
> char *p;
>
> sprintf(buf, "for");
> p = strchr(buf, 'f');
> if (p) *p = 'x';
>
> With your proposed change, the fifth line would need to be
> rewritten as:
>
> p = (char *) strchr(buf, 'f');
>
> A 'const' declarator in a parameter declaration does not mean
> that the respective argument must be a read-only object. It means
> that the function may not alter that argument. That is why
> ap_strchr_c should return a value of type char *.
Slow down there, Tex. The Single Unix Spec is broken in the sense that it
loses the const when we don't want it to. Sure, the *spec* is written that
way to enable your scenario. But that isn't what we want in Apache. We want
it to be as tight as possible.
This "looseness" in strchr() and strstr() turned up two "problems" in the
DAV code that was just added. Just declaration problems rather than true
"oops, I tried to modify a truly constant string", but I consider them
problems none-the-less. With Ryan's bread crumb for the right path, I
tracked them all(?) down.
For Apache, we have two functions:
char * ap_strchr(char *, int)
const char * ap_strchr_c(const char *, int)
Use the right one for the job.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: util_xml.c has constification warnings.ry
Posted by Sascha Schumann <sa...@schumann.cx>.
On Mon, 3 Jul 2000, Greg Stein wrote:
> On Sun, Jul 02, 2000 at 07:50:54PM -0700, rbb@covalent.net wrote:
> >
> > I just compiled the most recent code, and util_xml.c has some const
> > warnings from using strchr. In investigating this, I discovered that
> > ap_strchr_c is defined incorrectly. I am assuming that ap_strchr_c is
> > meant to conform to the Single Unix Spec exactly. We are defining it as
> >
> > const char * ap_strchr_c(const char *, int)
> >
> > Single Unix defines it as
> >
> > char * strchr(const char *, int)
> >
> > By changing our definition appropriately, and calling ap_strchr_c as in
> > the patch below, we remove all warnings from the server. Does anybody
> > mind if I commit this patch?
>
> The only reason that we have the ap_strchr_c() is because the Single Unix
> Spec is broken. It gives back a writable string when you feed it an
> *unwriteable* one. In other words, it is dropping the const and it really
> shouldn't.
Your analysis is implying that you can pass only read-only
strings to strchr. This is false.
The prototype was written that way to avoid casts. Imagine this:
char buf[256];
char *p;
sprintf(buf, "for");
p = strchr(buf, 'f');
if (p) *p = 'x';
With your proposed change, the fifth line would need to be
rewritten as:
p = (char *) strchr(buf, 'f');
A 'const' declarator in a parameter declaration does not mean
that the respective argument must be a read-only object. It means
that the function may not alter that argument. That is why
ap_strchr_c should return a value of type char *.
- Sascha
Re: util_xml.c has constification warnings.
Posted by Greg Stein <gs...@lyra.org>.
On Sun, Jul 02, 2000 at 07:50:54PM -0700, rbb@covalent.net wrote:
>
> I just compiled the most recent code, and util_xml.c has some const
> warnings from using strchr. In investigating this, I discovered that
> ap_strchr_c is defined incorrectly. I am assuming that ap_strchr_c is
> meant to conform to the Single Unix Spec exactly. We are defining it as
>
> const char * ap_strchr_c(const char *, int)
>
> Single Unix defines it as
>
> char * strchr(const char *, int)
>
> By changing our definition appropriately, and calling ap_strchr_c as in
> the patch below, we remove all warnings from the server. Does anybody
> mind if I commit this patch?
The only reason that we have the ap_strchr_c() is because the Single Unix
Spec is broken. It gives back a writable string when you feed it an
*unwriteable* one. In other words, it is dropping the const and it really
shouldn't.
Ben added those functions to debug the cases where we are feeding in const,
but using non-const on the outside. The ap_strchr_c() and brethren are to
catch these error cases.
In other words, the patch defeats their purpose :-(
Cheers,
-g
--
Greg Stein, http://www.lyra.org/