httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dean Gaudet <dgau...@arctic.org>
Subject Re: [PATCH] table_do has improper prototype
Date Tue, 01 Apr 1997 09:12:46 GMT
I really find it hard to believe that you intended all the side-effects of
allowing modifications to the table while table_do is running.  Consider
a call to table_unset from within the comp function -- table_do would
possibly miss some of the elements.  A call to table_add could change
t->elts, and table_do would be walking an old version of the table.
To allow in-place updates of a table requires cursors, which table_do
doesn't implement (thank god).

So if those modifications aren't permitted then all that's left is for
comp to play with the key and value, in place -- since it isn't passed
the table_entry *.  To allow comp to do that goes against this comment
from alloc.h:

/* Tables.  Implemented alist style, for now, though we try to keep
 * it so that imposing a hash table structure on top in the future
 * wouldn't be *too* hard...

which I happen to agree with ... especially since there are optimizations
we can do to tables without doing full hash lists.  (tables are frequently
used to answer the "does this exist" question, which is easy to answer
in the negative with a hash into a bitvector).

At any rate, regarding my patch.  It compiles cleanly under gcc 2.7.2
because I also changed the prototype of send_header_field so that the
cast was unnecessary.

Dean

On Tue, 1 Apr 1997, Roy T. Fielding wrote:

> >table_do takes an int(*)(void *, char *, char *) but those should be const
> >char * otherwise it violates the abstraction of tables (i.e. would break
> >if we made them into hash_tables).  Also the casting of send_header_field
> >in http_protocol.c overrides C's (meagre) type-checking.
> 
> Ummm, no it doesn't violate the abstraction of tables (only the key
> and value is passed) and the intention is that they be modifiable
> by the routine. const is not desirable in this case, and the casting
> is necessary to avoid warnings on my (2.7.2) gcc.
> 
> >Roy, this makes send_header_field a static... whereas it looks like you
> >intended it to be useable outside http_protocol.c.
> 
> Yes, I did (note the declaration in http_protocol.h).
> 
> >Are there really broken compilers that require you to (void *)r when
> >passing to a prototyped void * parameter?
> 
> Yes.
> 
> .....Roy  (-1)
> 


Mime
View raw message