Date: Wed, 23 Jul 1997 22:21:54 -0700 (PDT)
From: Dean Gaudet <dgaudet@arctic.org>
To: new-httpd@apache.org
Subject: Re: [PATCH] Enable sortable columns in FancyIndexed directories
In-Reply-To: <97072310424897@decus.org>
Message-ID: <Pine.LNX.3.95dg3.970723221542.7392G-100000@twinlark.arctic.org>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: new-httpd-owner@apache.org
Precedence: bulk
Reply-To: new-httpd@apache.org

I haven't tested this, but it looks fine except:

On Wed, 23 Jul 1997, Rodent of Unusual Size wrote:

> +static void emit_link(request_rec *r, char *anchor, char fname, char curkey,
> +		      char curdirection)
> +{
> +    char *qvalue = "?.=.";
> +    int reverse;
> +
> +    qvalue[1] = fname;
> +    reverse = ((curkey == fname) && (curdirection == D_ASCENDING));
> +    qvalue[3] = reverse ? D_DESCENDING : D_ASCENDING;
> +    rvputs (r, "<A HREF=\"", qvalue, "\">", anchor, "</A>", NULL);
> +}

qvalue points to a const char *... which most compilers don't warn about
because it's such a common problem.  Your code here isn't thread safe.
It should be something like:

    char qvalue[5];

    qvalue[0] = '?';
    qvalue[1] = fname;
    ....
    qvalue[4] = 0;  /* just to annoy Marc */

If it were a really long string you could use snprintf to fill it in,
or whatever... you just need to be sure you're not modifying a statically
allocated string.

gcc -Wwrite-strings warns about this sort of thing ... but is really hard
to use against apache at the moment.  It's probably worthwhile for us to
take this step at some point ... there could be subtle threading bugs
like this above.

Dean