You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Rodent of Unusual Size <co...@decus.org> on 1997/07/23 16:42:49 UTC

[PATCH] Enable sortable columns in FancyIndexed directories

    I'm still struggling with the IndexFormat stuff, so I took a break
    and did this.

    Ta-daah!  Sortable FancyIndexed directories.  Click on the column
    heading, and the listing is sorted by that field.  Clicking on a
    column header repeatedly toggles the direction, ascending or
    descending, of the sort.  (That is, click on "Last modified" when
    the listing is sorted by file name [the default], and it will be
    sorted by ascending LM date.  Click "Last modified" again, and it's
    sorted by DEscending LM date.  Click it a third time in a row, and
    it's ascending again.)

    If there are duplicate values for a key (such as "Description"), the
    duplicates are subsorted by ascending filename.

    Note that sort-by-size uses the *actual* file size to all
    significant digits, *not* the displayed size.

    Lazy voting mode assumed.. no remarks requiring changes w/in 48
    hours, and I commit it to HEAD.  No attempts have been made to clean
    up TABs.

    #ken    :-)}

Index: mod_autoindex.c
===================================================================
RCS file: /export/home/cvs/apache/src/mod_autoindex.c,v
retrieving revision 1.40
diff -c -u -r1.40 mod_autoindex.c
/usr/bin/diff: conflicting specifications of output style
--- mod_autoindex.c	1997/07/21 05:53:49	1.40
+++ mod_autoindex.c	1997/07/23 14:38:00
@@ -89,6 +89,17 @@
 #define SUPPRESS_PREAMBLE 64
 
 /*
+ * Define keys for sorting.
+ */
+#define K_NAME 'N'		/* Sort by file name (default) */
+#define K_LAST_MOD 'M'		/* Last modification date */
+#define K_SIZE 'S'		/* Size (absolute, not as displayed) */
+#define K_DESC 'D'		/* Description */
+
+#define D_ASCENDING 'A'
+#define D_DESCENDING 'D'
+
+/*
  * These are the dimensions of the default icons supplied with Apache.
  */
 #define DEFAULT_ICON_WIDTH 20
@@ -370,8 +381,12 @@
     char *alt;
     char *desc;
     size_t size;
+    char *size_cmp;
     time_t lm;
+    char *lm_cmp;
     struct ent *next;
+    int ascending;
+    char key;
 };
 
 static char *find_item(request_rec *r, array_header *list, int path_only) {
@@ -593,7 +608,9 @@
 }
 
 static struct ent *make_autoindex_entry(char *name, int autoindex_opts,
-                           autoindex_config_rec *d, request_rec *r)
+					autoindex_config_rec *d,
+					request_rec *r, char keyid,
+					char direction)
 {
     struct ent *p;
 
@@ -610,6 +627,8 @@
     p->alt = NULL;
     p->desc = NULL;
     p->lm = -1;
+    p->key = toupper(keyid);
+    p->ascending = (toupper(direction) == D_ASCENDING);
 
     if (autoindex_opts & FANCY_INDEXING) {
         request_rec *rr = sub_req_lookup_file (name, r);
@@ -638,6 +657,16 @@
 
         destroy_sub_req (rr);
     }
+    if (keyid == K_SIZE) {
+	p->size_cmp = pcalloc (r->pool, 14);
+	ap_snprintf (p->size_cmp, 14, "%013d", p->size);
+    }
+    if (keyid == K_LAST_MOD) {
+	struct tm *ts = localtime(&p->lm);
+
+	p->lm_cmp = pcalloc (r->pool, 15);
+	strftime (p->lm_cmp, 14, "%Y%m%d%H%M%S", ts);
+    }
     return(p);
 }
 
@@ -669,8 +698,26 @@
     return desc;
 }
 
+/*
+ * Emit the anchor for the specified field.  If a field is the key for the
+ * current request, the link changes its meaning to reverse the order when
+ * selected again.  Non-active fields always start in ascending order.
+ */
+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);
+}
+
 static void output_directories(struct ent **ar, int n,
-    autoindex_config_rec *d, request_rec *r, int autoindex_opts)
+			       autoindex_config_rec *d, request_rec *r,
+			       int autoindex_opts, char keyid, char direction)
 {
     int x, len;
     char *name = r->uri;
@@ -695,13 +742,19 @@
             }
             rputs ("> ", r);
         }
-        rputs("Name                   ", r);
-        if (!(autoindex_opts & SUPPRESS_LAST_MOD))
-            rputs("Last modified     ", r);
-        if (!(autoindex_opts & SUPPRESS_SIZE))
-            rputs("Size  ", r);
-        if (!(autoindex_opts & SUPPRESS_DESC))
-            rputs("Description", r);
+	emit_link(r, "Name", K_NAME, keyid, direction);
+	rputs ("                   ", r);
+        if (!(autoindex_opts & SUPPRESS_LAST_MOD)) {
+	    emit_link(r, "Last modified", K_LAST_MOD, keyid, direction);
+            rputs("     ", r);
+	}
+        if (!(autoindex_opts & SUPPRESS_SIZE)) {
+	    emit_link(r, "Size", K_SIZE, keyid, direction);
+            rputs("  ", r);
+	}
+        if (!(autoindex_opts & SUPPRESS_DESC)) {
+	    emit_link(r, "Description", K_DESC, keyid, direction);
+	}
         rputs("\n<HR>\n", r);
     }
     else {
@@ -802,9 +855,81 @@
 }
 
 
-static int dsortf(struct ent **s1, struct ent **s2)
+static int dsortf(struct ent **e1, struct ent **e2)
 {
-    return(strcmp((*s1)->name, (*s2)->name));
+    char *s1;
+    char *s2;
+    char *s3;
+    int result;
+
+    /*
+     * Choose the right values for the sort keys.
+     */
+    switch ((*e1)->key) {
+    case K_LAST_MOD:
+	s1 = (*e1)->lm_cmp;
+	s2 = (*e2)->lm_cmp;
+	break;
+    case K_SIZE:
+	s1 = (*e1)->size_cmp;
+	s2 = (*e2)->size_cmp;
+	break;
+    case K_DESC:
+	s1 = (*e1)->desc;
+	s2 = (*e2)->desc;
+	break;
+    case K_NAME:
+    default:
+	s1 = (*e1)->name;
+	s2 = (*e2)->name;
+	break;
+    }
+    /*
+     * If we're supposed to sort in DEscending order, reverse the arguments.
+     */
+    if (!(*e1)->ascending) {
+	s3 = s1;
+	s1 = s2;
+	s2 = s3;
+    }
+
+    /*
+     * Take some care, here, in case one string or the other (or both) is
+     * NULL.
+     */
+
+    /*
+     * Two valid strings, compare normally.
+     */
+    if ((s1 != NULL) && (s2 != NULL)) {
+	result = strcmp(s1, s2);
+    }
+    /*
+     * Two NULL strings - primary keys are equal (fake it).
+     */
+    else if ((s1 == NULL) && (s2 == NULL)) {
+	result = 0;
+    }
+    /*
+     * s1 is NULL, but s2 is a string - so s2 wins.
+     */
+    else if (s1 == NULL) {
+	result = -1;
+    }
+    /*
+     * Last case: s1 is a string and s2 is NULL, so s1 wins.
+     */
+    else {
+	result = 1;
+    }
+    /*
+     * If the keys were equal, the file name is *always* the secondary key -
+     * in ascending order.
+     */
+    if (!result) {
+	result = strcmp((*e1)->name, (*e2)->name);
+    }
+    return result;
 }
 
     
@@ -820,7 +945,13 @@
     struct ent *head, *p;
     struct ent **ar = NULL;
     char *tmp;
+    const char *qstring;
     int autoindex_opts = find_opts(autoindex_conf, r);
+    char key;
+    int sort_ascending = 1;
+    char *query_string;
+    char keyid;
+    char direction;
 
     if (!(d = popendir(r->pool, name))) {
         log_reason ("Can't open directory for index", r->filename, r);
@@ -852,13 +983,39 @@
         rvputs(r, "<H1>Index of ", title_name, "</H1>\n", NULL);
     }
 
+    /*
+     * Figure out what sort of indexing (if any) we're supposed to use.
+     */
+    qstring = r->args;
+
+    /*
+     * If no QUERY_STRING was specified, we use the default: ascending  by
+     * name.
+     */
+    if ((qstring == NULL) || (*qstring == '\0')) {
+	keyid = K_NAME;
+	direction = D_ASCENDING;
+    }
+    else {
+	keyid = *qstring;
+	getword (r->pool, &qstring, '=');
+	if (qstring != '\0') {
+	    direction = *qstring;
+	}
+	else {
+	    direction = D_ASCENDING;
+	}
+    }
+
     /* 
      * Since we don't know how many dir. entries there are, put them into a 
      * linked list and then arrayificate them so qsort can use them. 
      */
     head = NULL;
     while ((dstruct = readdir(d))) {
-        if ((p = make_autoindex_entry(dstruct->d_name, autoindex_opts, autoindex_conf, r))) {
+        p = make_autoindex_entry(dstruct->d_name, autoindex_opts,
+				 autoindex_conf, r, keyid, direction);
+	if (p != NULL) {
             p->next = head;
             head = p;
             num_ent++;
@@ -880,7 +1037,8 @@
               (int (*)(const void *, const void *))dsortf);
 #endif
     }
-    output_directories(ar, num_ent, autoindex_conf, r, autoindex_opts);
+    output_directories(ar, num_ent, autoindex_conf, r, autoindex_opts, keyid,
+		       direction);
     pclosedir(r->pool, d);
 
     if (autoindex_opts & FANCY_INDEXING)

Re: [PATCH] Enable sortable columns in FancyIndexed directories

Posted by Dean Gaudet <dg...@arctic.org>.

On Wed, 23 Jul 1997, Alexei Kosut wrote:

> Or just
> 
> char qvalue = pstrdup(r->pool, "?.=.");
> 
> The Apache code actually uses that sort of construct a lot, probably
> uneccessarily in a lot of places. 

This is one of those unnecessary places.  Why allocate something that
lives beyond the end of the function when it's trivial to allocate it on
the stack?  (without imposing some compile time limitation on string
length) 

> But in this instance, this should work too (I think):
> 
> char qvalue[] = "?.=.";

This would perform the equivalent of a memcpy each time the function is
entered.  Whereas the method I gave initializes each element only once. 

Dean - performance freak



Re: [PATCH] Enable sortable columns in FancyIndexed directories

Posted by Alexei Kosut <ak...@organic.com>.
On Wed, 23 Jul 1997, Dean Gaudet wrote:

> 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:

Or just

char qvalue = pstrdup(r->pool, "?.=.");

The Apache code actually uses that sort of construct a lot, probably
uneccessarily in a lot of places.

But in this instance, this should work too (I think):

char qvalue[] = "?.=.";

-- Alexei Kosut <ak...@organic.com>


Re: [PATCH] Enable sortable columns in FancyIndexed directories

Posted by Dean Gaudet <dg...@arctic.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


Re: [PATCH] Enable sortable columns in FancyIndexed directories

Posted by Marc Slemko <ma...@worldgate.com>.
Haven't done anything other than a quick read of the code yet...

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

> Index: mod_autoindex.c
> ===================================================================
> RCS file: /export/home/cvs/apache/src/mod_autoindex.c,v
> retrieving revision 1.40
> diff -c -u -r1.40 mod_autoindex.c
> /usr/bin/diff: conflicting specifications of output style

While a conflict and unidiff at the same time would be... erm...
interesting, diff don't do that.  <g>

> +    if (keyid == K_SIZE) {
> +	p->size_cmp = pcalloc (r->pool, 14);
> +	ap_snprintf (p->size_cmp, 14, "%013d", p->size);
> +    }
> +    if (keyid == K_LAST_MOD) {
> +	struct tm *ts = localtime(&p->lm);
> +
> +	p->lm_cmp = pcalloc (r->pool, 15);
> +	strftime (p->lm_cmp, 14, "%Y%m%d%H%M%S", ts);

Why 15 for pcalloc and 14 for strftime?

Why use pcalloc instead of palloc?