Re: [RFA] Sorting symbols. Again.

[Syd Polk <spolk at apple dot com>]

Where is sort_funcVals called from? It it called pretty close to the Tcl 
layer? If so, you might want to use "lsort -command foo" from the tcl 
level and implement the comparison command in C. Given that the sort 
command you are using pares down to a simple strcmp, this would be much 
cleaner than doing what you are doing.

> Index: gdbtk/generic/gdbtk-cmds.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbtk/generic/gdbtk-cmds.c,v
> retrieving revision 1.48
> diff -u -p -r1.48 gdbtk-cmds.c
> --- gdbtk-cmds.c	2002/01/08 20:21:44	1.48
> +++ gdbtk-cmds.c	2002/01/30 05:42:36
> @@ -1452,6 +1452,19 @@ gdb_search (clientData, interp, objc, ob
>    return TCL_OK;
>  }
>
> +static int
> +sort_funcVals (const void *fa, const void *fb)
> +{
> +  Tcl_Obj *funcVal_a = *(Tcl_Obj **)fa;
> +  Tcl_Obj *funcVal_b = *(Tcl_Obj **)fb;
> +  Tcl_Obj *func_a, *func_b;
> +
> +  Tcl_ListObjIndex (NULL, funcVal_a, 0, &func_a);
> +  Tcl_ListObjIndex (NULL, funcVal_b, 0, &func_b);
> +
> +  return strcmp (Tcl_GetString (func_a), Tcl_GetString (func_b));
> +}
> +
>  /* This implements the tcl command gdb_listfuncs
>
>   * It lists all the functions defined in a given file
> @@ -1477,6 +1490,8 @@ gdb_listfuncs (clientData, interp, objc,
>    struct symbol *sym;
>    int i, j;
>    Tcl_Obj *funcVals[2];
> +  int list_objc;
> +  Tcl_Obj **list_objv, **new_objv;
>
>    if (objc != 2)
>      {
> @@ -1506,9 +1521,6 @@ gdb_listfuncs (clientData, interp, objc,
>    for (i = GLOBAL_BLOCK; i <= STATIC_BLOCK; i++)
>      {
>        b = BLOCKVECTOR_BLOCK (bv, i);
> -      /* Skip the sort if this block is always sorted.  */
> -      if (!BLOCK_SHOULD_SORT (b))
> -	sort_block_syms (b);
>        ALL_BLOCK_SYMBOLS (b, j, sym)
>  	{
>  	  if (SYMBOL_CLASS (sym) == LOC_BLOCK)
> @@ -1544,6 +1556,17 @@ gdb_listfuncs (clientData, interp, objc,
>  					Tcl_NewListObj (2, funcVals));
>  	    }
>  	}
> +      Tcl_ListObjGetElements (NULL, result_ptr->obj_ptr, &list_objc, 
> &list_objv);
> +      new_objv = (Tcl_Obj **) malloc (sizeof (Tcl_Obj *) * list_objc);
> +      memcpy (new_objv, list_objv, sizeof (Tcl_Obj *) * list_objc);
> +      qsort (new_objv, list_objc, sizeof (Tcl_Obj *), sort_funcVals);
> +      for (j = 0; j < list_objc; j++)
> +	Tcl_IncrRefCount (list_objv[j]);
> +      Tcl_SetListObj (result_ptr->obj_ptr, list_objc, new_objv);
> +      Tcl_ListObjGetElements (NULL, result_ptr->obj_ptr, &list_objc, 
> &list_objv);
> +      for (j = 0; j < list_objc; j++)
> +	Tcl_DecrRefCount (list_objv[j]);
> +      free (new_objv);

You should not have to loop through the objects in list_objv to Incr or 
Decr their refCounts. Tcl_SetListObj does that automatically. They are 
created with a refCount of 0; Tcl_SetListObj incrs them to 1. What you 
are doing is creating them, setting them to 1, calling Tcl_SetListObj 
(which incrs them to 2), and the decrementing them back to 1.

Also, it is really, really slimy to create Tcl_Objs without going 
through Tcl_New*Obj.

I would much prefer that you duplicate the list, and then call "lsort". 
Pseudo-code:

	Tcl_Obj *commandArray[2];

	Tcl_ListObjGetElements (NULL, result_ptr->obj_ptr, &list_objc, 
&list_objv);
	newList = Tcl_NewListObj(list_objc, list_objv);
	commandArray[1] = newList;
	Tcl_IncrRefCount(newList);
	commandArray[0] = Tcl_NewObjFromString("lsort");
	Tcl_IncrRefCount(commandArray[0]);
	result = Tcl_EvalObjv(interp, 2, commandArray);
	Tcl_DecrRefCount(commandArray[0]);
	
	/* newList now has sorted list. */

I am not a maintainer, but I worked in the core of Tcl for a couple of 
years for John O., and doing block allocates of Tcl_Objs is just asking 
for trouble, and will lead to problems if insight is ever compiled to 
TCL_MEM_DEBUG, or other things.


>      }
>    return TCL_OK;
>  }
>
>
Syd Polk
QA and Integration Manager, Mac OS X Development Tools
+1 408 974-0577