From new-httpd-owner@apache.org Tue Apr 1 01:12:51 1997 Received: by taz.hyperreal.com (8.8.4/V2.0) id BAA00547; Tue, 1 Apr 1997 01:12:51 -0800 (PST) Received: from twinlark.arctic.org by taz.hyperreal.com (8.8.4/V2.0) with SMTP id BAA00542; Tue, 1 Apr 1997 01:12:48 -0800 (PST) Received: (qmail 9564 invoked by uid 500); 1 Apr 1997 09:12:46 -0000 Date: Tue, 1 Apr 1997 01:12:46 -0800 (PST) From: Dean Gaudet To: new-httpd@hyperreal.com Subject: Re: [PATCH] table_do has improper prototype In-Reply-To: <9704010057.aa03247@paris.ics.uci.edu> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: new-httpd-owner@apache.org Precedence: bulk Reply-To: new-httpd@hyperreal.com 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) >