httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <trawi...@bellsouth.net>
Subject Re: [PATCH] (big) buff.c/buff.h translation cleanup
Date Fri, 12 May 2000 02:04:39 GMT
It seems that nobody else had any comments, but Greg Ames and I
discussed a cleanup that I'll make before I commit...

> 1) when the configuration supports translation (i.e., APACHE_XLATE is
>    defined), some function pointers are used for key buff operations
>    (current ap_bread() and ap_bwrite()) so that translation code can
>    be separated from buffer management code and so that the code
>    doesn't have to check for whether or not it should translating
> 
>    e.g., when translation is on, ap_bwrite() uses a pointer in BUFF
>    which points to ap_bwrite_xlate(), which always does translation;
>    it calls ap_bwrite_core(), which never does translation; when
>    translation is off, the pointer in BUFF used by ap_bwrite() points
>    directly to ap_bwrite_core()
> 
>    As far as I can tell, this is one of Martin's ideas posted recently
>    for how to clean up the translation.
...
> +#ifdef APACHE_XLATE
> +typedef struct biol {
> +    ap_status_t (*bwrite)(BUFF *, const void *, ap_size_t, ap_ssize_t *);
> +    ap_status_t (*bread)(BUFF *, const void *, ap_size_t, ap_ssize_t *);
                                     .
                                    /|\
                                     |
                                     |
         bugola - shouldn't be const; I guess I dumbed-down the 
                    OS/390 compiler a bit too much
> +} biol;
> +#endif /*APACHE_XLATE*/
> +
...
> +#ifdef APACHE_XLATE
> +    biol biol;

This structure doesn't represent the fact that (assuming we add more
function pointers in the future) the read operations are modified
independently of the write operations, because the two translation
possibilities are independent.  What is really needed is something
like the following:

#ifdef APACHE_XLATE
typedef struct b_read_ops {
    ap_status_t (*bread)(BUFF *, void *, ap_size_t, ap_ssize_t *);
} b_read_ops;

typedef struct b_write_ops {
    ap_status_t (*bwrite)(BUFF *, const void *, ap_size_t, ap_ssize_t *);
} b_write_ops;
#endif

...

struct buff_struct {

  (existing stuff)

#ifdef APACHE_XLATE
    b_read_ops b_read_ops;
    b_write_ops b_write_ops;
    (more stuff)
#endif
};

and in buff.c, declare a static structure for each with the right
contents, and do structure assignment when modifying the BO_WXLATE or
BO_RXLATE options

#ifdef APACHE_XLATE
static const b_read_ops binary_read_ops = {ap_bread_core};
static const b_read_ops xlate_read_ops  = {bread_xlate};
static const b_write_ops binary_write_ops = {ap_bwrite_core};
static const b_write_ops xlate_write_ops  = {bwrite_xlate};
#endif

> minor issues addressed:
> 
> 1) 1.3 translation support kept a single buffer per httpd process for
>    translating in ap_bwrite(); now there is one buffer per BUFF so
>    that it is thread-safe
...
> +    if (nbyte > fb->xbufsize) {
> +        if (fb->xbuf != NULL) {
> +            free(fb->xbuf);
> +        }
> +        fb->xbufsize = (nbyte + HUGE_STRING_LEN + 1023) & ~1023;
> +        fb->xbuf = (char *)malloc(fb->xbufsize);
> +        ap_assert(fb->xbuf);
> +    }

Greg suggests using ap_palloc() instead of malloc().  That sounds
correct, but I'll wait on this one.  (It will be useful to get a feel
for what buffer sizes are actually needed in practice.)

> impacts to non-translating configurations (i.e., when neither
> APACHE_XLATE or CHARSET_EBCDIC is defined):
> 
> . no extra storage
> . no extra pathlength
> . no different code executed
> . ap_bread() and ap_bwrite() are macros which map to ap_bread_core()
>   and ap_bwrite_core()

I could leave ap_bread_core() as ap_bread() and leave ap_bwrite_core()
as ap_bwrite().  Any opinions?

> +#else /*APACHE_XLATE*/
> + 
> +/* can only unput a single character that was read by ap_bgetc */
                 .
                /|\
                 |
                 |
         typo; should be unget

-- 
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
     http://www.geocities.com/SiliconValley/Park/9289/
          Born in Roswell... married an alien...

Mime
View raw message