> If I recall it was painful to find entries in the help listing w/o sorting.
So it's a human reading problem, where ddb spat out the command names in order that they were in the in-memory struct, and if I wanted to look over the listing I had to visually scan every one since they weren't in order? > Feel free to propose an alternative implementation; noone's wed to > what's in the tree. Here's a patch for 6.x. The display order for the commands is an interesting issue; it would also make sense to group commands (perhaps alphabetically) by the module that defined them, so that when I added a module all my ddb commands stayed together. The patch I have here will leave commands grouped by module, so it would be relatively trivial to have linker_file_register_ddb add a new kind of dummy command that specifies the module name for the following set of commands. N.B. It appears from the FAQ that attachments are acceptable so I'm sending the patch as an attachment. Apologies if this is incorrect. Thanks, matthew
Index: kern/kern_linker.c =================================================================== --- kern/kern_linker.c (revision 108659) +++ kern/kern_linker.c (working copy) @@ -40,43 +40,53 @@ __FBSDID("$FreeBSD: src/sys/kern/kern_li #include <sys/proc.h> #include <sys/lock.h> #include <sys/mutex.h> #include <sys/sx.h> #include <sys/mac.h> #include <sys/module.h> #include <sys/linker.h> #include <sys/fcntl.h> #include <sys/libkern.h> #include <sys/namei.h> #include <sys/vnode.h> #include <sys/sysctl.h> #ifdef KDB #include <sys/kdb.h> #endif +#ifdef DDB +#include <ddb/ddb.h> +#endif /* DDB */ #include <vm/uma.h> #include "linker_if.h" #ifdef KLD_DEBUG int kld_debug = 0; #endif /* * static char *linker_search_path(const char *name, struct mod_depend * *verinfo); */ static const char *linker_basename(const char *path); +#ifdef DDB +static void linker_file_register_ddb(linker_file_t file); +static void linker_file_unregister_ddb(linker_file_t file); +#else /* !... DDB */ +#define linker_file_register_ddb(file) do { } while (0) +#define linker_file_unregister_ddb(file) do { } while (0) +#endif /* DDB */ /* Metadata from the static kernel */ SET_DECLARE(modmetadata_set, struct mod_metadata); MALLOC_DEFINE(M_LINKER, "linker", "kernel linker"); linker_file_t linker_kernel_file; static struct mtx kld_mtx; /* kernel linker mutex */ static linker_class_list_t classes; linker_file_list_t linker_files; static int next_file_id = 1; static int linker_no_more_classes = 0; @@ -361,30 +371,36 @@ linker_load_file(const char *filename, l KLD_DPF(FILE, ("linker_load_file: trying to load %s\n", filename)); error = LINKER_LOAD_FILE(lc, filename, &lf); /* * If we got something other than ENOENT, then it exists but * we cannot load it for some other reason. */ if (error != ENOENT) foundfile = 1; if (lf) { error = linker_file_register_modules(lf); if (error == EEXIST) { linker_file_unload(lf, LINKER_UNLOAD_FORCE); goto out; } + /* + * Register ddb commands before anything, so + * they're available in case sysinit bug + * causes a crash. + */ + linker_file_register_ddb(lf); linker_file_register_sysctls(lf); linker_file_sysinit(lf); lf->flags |= LINKER_FILE_LINKED; *result = lf; error = 0; goto out; } } /* * Less than ideal, but tells the user whether it failed to load or * the module was not found. */ if (foundfile) { /* * Format not recognized or otherwise unloadable. @@ -540,30 +556,31 @@ linker_file_unload(linker_file_t file, i for (ml = TAILQ_FIRST(&found_modules); ml; ml = nextml) { nextml = TAILQ_NEXT(ml, link); if (ml->container == file) { TAILQ_REMOVE(&found_modules, ml, link); free(ml, M_LINKER); } } /* * Don't try to run SYSUNINITs if we are unloaded due to a * link error. */ if (file->flags & LINKER_FILE_LINKED) { linker_file_sysuninit(file); linker_file_unregister_sysctls(file); + linker_file_unregister_ddb(file); } mtx_lock(&kld_mtx); TAILQ_REMOVE(&linker_files, file, link); mtx_unlock(&kld_mtx); if (file->deps) { for (i = 0; i < file->ndeps; i++) linker_file_unload(file->deps[i], flags); free(file->deps, M_LINKER); file->deps = NULL; } for (cp = STAILQ_FIRST(&file->common); cp; cp = STAILQ_FIRST(&file->common)) { STAILQ_REMOVE(&file->common, cp, common_symbol, link); free(cp, M_LINKER); @@ -742,31 +759,74 @@ linker_ddb_search_symbol(caddr_t value, return (ENOENT); } } int linker_ddb_symbol_values(c_linker_sym_t sym, linker_symval_t *symval) { linker_file_t lf; TAILQ_FOREACH(lf, &linker_files, link) { if (LINKER_SYMBOL_VALUES(lf, sym, symval) == 0) return (0); } return (ENOENT); } -#endif + +/* + * Add and remove db_cmd and db_show_cmd that are defined in a module. + */ +static void +linker_file_register_ddb(linker_file_t file) +{ + struct command **c_start, **c_stop; + char *set; + + set = "db_cmd_set"; + if (linker_file_lookup_set(file, set, &c_start, &c_stop, NULL) == 0) { + if (db_cmd_add(c_start, c_stop, false) != 0) { + printf("Could not allocate memory to add to %s; " + "module %s's db commands are not merged\n", + set, file->filename); + } + } + + set = "db_show_cmd_set"; + if (linker_file_lookup_set(file, set, &c_start, &c_stop, NULL) == 0) { + if (db_cmd_add(c_start, c_stop, true) != 0) { + printf("Could not allocate memory to add to %s; " + "module %s's db commands are not merged\n", + set, file->filename); + } + } +} + +static void +linker_file_unregister_ddb(linker_file_t file) +{ + struct command **c_start, **c_stop; + + if (linker_file_lookup_set(file, "db_cmd_set", &c_start, + &c_stop, NULL) == 0) + db_cmd_rem(c_start, c_stop, false); + + if (linker_file_lookup_set(file, "db_show_cmd_set", &c_start, + &c_stop, NULL) == 0) + db_cmd_rem(c_start, c_stop, true); +} + +#endif /* DDB */ /* * Syscalls. */ /* * MPSAFE */ int kldload(struct thread *td, struct kldload_args *uap) { char *kldname, *modname; char *pathname = NULL; linker_file_t lf; int error = 0; size_t size; @@ -1552,30 +1612,31 @@ restart: } } /* * Now do relocation etc using the symbol search paths * established by the dependencies */ error = LINKER_LINK_PRELOAD_FINISH(lf); if (error) { printf("KLD file %s - could not finalize loading\n", lf->filename); TAILQ_REMOVE(&depended_files, lf, loaded); linker_file_unload(lf, LINKER_UNLOAD_FORCE); continue; } linker_file_register_modules(lf); + linker_file_register_ddb(lf); if (linker_file_lookup_set(lf, "sysinit_set", &si_start, &si_stop, NULL) == 0) sysinit_add(si_start, si_stop); linker_file_register_sysctls(lf); lf->flags |= LINKER_FILE_LINKED; } /* woohoo! we made it! */ } SYSINIT(preload, SI_SUB_KLD, SI_ORDER_MIDDLE, linker_preload, 0) /* * Search for a not-loaded module by name. * * Modules may be found in the following locations: Index: ddb/ddb.h =================================================================== --- ddb/ddb.h (revision 108659) +++ ddb/ddb.h (working copy) @@ -77,60 +77,63 @@ func_name(addr, have_addr, count, modif) db_expr_t count; \ char *modif; extern db_expr_t db_maxoff; extern int db_indent; extern int db_inst_count; extern int db_load_count; extern int db_store_count; extern db_expr_t db_radix; extern db_expr_t db_max_width; extern db_expr_t db_tab_stop_width; extern db_expr_t db_lines_per_page; struct thread; struct vm_map; +struct command; void db_check_interrupt(void); void db_clear_watchpoints(void); db_addr_t db_disasm(db_addr_t loc, boolean_t altfmt); /* instruction disassembler */ void db_error(const char *s); int db_expression(db_expr_t *valuep); int db_get_variable(db_expr_t *valuep); void db_iprintf(const char *,...) __kprintflike(1, 2); struct vm_map *db_map_addr(vm_offset_t); boolean_t db_map_current(struct vm_map *); boolean_t db_map_equal(struct vm_map *, struct vm_map *); void db_print_loc_and_inst(db_addr_t loc); void db_print_thread(void); void db_printf(const char *fmt, ...) __kprintflike(1, 2); int db_read_bytes(vm_offset_t addr, size_t size, char *data); /* machine-dependent */ int db_readline(char *lstart, int lsize); void db_restart_at_pc(boolean_t watchpt); int db_set_variable(db_expr_t value); void db_set_watchpoints(void); void db_setup_paging(db_page_calloutfcn_t *callout, void *arg, int maxlines); void db_skip_to_eol(void); boolean_t db_stop_at_pc(boolean_t *is_breakpoint); #define db_strcpy strcpy void db_trace_self(void); int db_trace_thread(struct thread *, int); int db_value_of_name(const char *name, db_expr_t *valuep); int db_write_bytes(vm_offset_t addr, size_t size, char *data); +int db_cmd_add(struct command **, struct command **, boolean_t); +void db_cmd_rem(struct command **, struct command **, boolean_t); db_cmdfcn_t db_breakpoint_cmd; db_cmdfcn_t db_continue_cmd; db_cmdfcn_t db_delete_cmd; db_cmdfcn_t db_deletehwatch_cmd; db_cmdfcn_t db_deletewatch_cmd; db_cmdfcn_t db_examine_cmd; db_cmdfcn_t db_hwatchpoint_cmd; db_cmdfcn_t db_listbreak_cmd; db_cmdfcn_t db_print_cmd; db_cmdfcn_t db_ps; db_cmdfcn_t db_search_cmd; db_cmdfcn_t db_set_cmd; db_cmdfcn_t db_set_thread; db_cmdfcn_t db_show_regs; Index: ddb/db_command.c =================================================================== --- ddb/db_command.c (revision 108659) +++ ddb/db_command.c (working copy) @@ -33,51 +33,60 @@ #include <sys/cdefs.h> __FBSDID("$FreeBSD: src/sys/ddb/db_command.c,v 1.60.2.2 2005/10/25 20:10:56 jhb Exp $"); #include <sys/param.h> #include <sys/linker_set.h> #include <sys/lock.h> #include <sys/kdb.h> #include <sys/mutex.h> #include <sys/proc.h> #include <sys/reboot.h> #include <sys/signalvar.h> #include <sys/systm.h> #include <sys/cons.h> #include <sys/watchdog.h> +#include <sys/malloc.h> #include <ddb/ddb.h> #include <ddb/db_command.h> #include <ddb/db_lex.h> #include <ddb/db_output.h> #include <machine/cpu.h> #include <machine/setjmp.h> /* * Exported global variables */ boolean_t db_cmd_loop_done; db_addr_t db_dot; db_addr_t db_last_addr; db_addr_t db_prev; db_addr_t db_next; SET_DECLARE(db_cmd_set, struct command); SET_DECLARE(db_show_cmd_set, struct command); + +struct command **db_cmd_start = SET_BEGIN(db_cmd_set); +struct command **db_cmd_end = SET_LIMIT(db_cmd_set); +struct command **db_show_cmd_start = SET_BEGIN(db_show_cmd_set); +struct command **db_show_cmd_end = SET_LIMIT(db_show_cmd_set); +int db_cmd_is_mallocd = 0; +int db_show_cmd_is_mallocd = 0; + static db_cmdfcn_t db_fncall; static db_cmdfcn_t db_gdb; static db_cmdfcn_t db_kill; static db_cmdfcn_t db_reset; static db_cmdfcn_t db_stack_trace; static db_cmdfcn_t db_stack_trace_all; static db_cmdfcn_t db_watchdog; /* XXX this is actually forward-static. */ extern struct command db_show_cmds[]; /* * if 'ed' style: 'dot' is set at start of last item printed, * and '+' points to next line. * Otherwise: 'dot' points to next item, '..' points to last. @@ -154,30 +163,33 @@ db_cmd_search(name, table, aux_tablep, a this lets us match single letters */ } else { *cmdp = cmd; result = CMD_FOUND; } } } if (aux_tablep != 0) /* XXX repeat too much code. */ for (aux_cmdp = aux_tablep; aux_cmdp < aux_tablep_end; aux_cmdp++) { register char *lp; register char *rp; register int c; + if (*aux_cmdp == NULL) + continue; + lp = name; rp = (*aux_cmdp)->name; while ((c = *lp) == *rp) { if (c == 0) { /* complete match */ *cmdp = *aux_cmdp; return (CMD_UNIQUE); } lp++; rp++; } if (c == 0) { /* end of name, not end of command - partial match */ if (result == CMD_FOUND) { @@ -204,30 +216,32 @@ static void db_cmd_list(table, aux_tablep, aux_tablep_end) struct command *table; struct command **aux_tablep; struct command **aux_tablep_end; { register struct command *cmd; register struct command **aux_cmdp; for (cmd = table; cmd->name != 0; cmd++) { db_printf("%-12s", cmd->name); db_end_line(); } if (aux_tablep == 0) return; for (aux_cmdp = aux_tablep; aux_cmdp < aux_tablep_end; aux_cmdp++) { + if (*aux_cmdp == NULL) + continue; db_printf("%-12s", (*aux_cmdp)->name); db_end_line(); } } static void db_command(last_cmdp, cmd_table, aux_cmd_tablep, aux_cmd_tablep_end) struct command **last_cmdp; /* IN_OUT */ struct command *cmd_table; struct command **aux_cmd_tablep; struct command **aux_cmd_tablep_end; { struct command *cmd; int t; char modif[TOK_STRING_SIZE]; @@ -271,32 +285,32 @@ db_command(last_cmdp, cmd_table, aux_cmd case CMD_AMBIGUOUS: db_printf("Ambiguous\n"); db_flush_lex(); return; case CMD_HELP: db_cmd_list(cmd_table, aux_cmd_tablep, aux_cmd_tablep_end); db_flush_lex(); return; default: break; } if ((cmd_table = cmd->more) != 0) { /* XXX usually no more aux's. */ aux_cmd_tablep = 0; if (cmd_table == db_show_cmds) { - aux_cmd_tablep = SET_BEGIN(db_show_cmd_set); - aux_cmd_tablep_end = SET_LIMIT(db_show_cmd_set); + aux_cmd_tablep = db_show_cmd_start; + aux_cmd_tablep_end = db_show_cmd_end; } t = db_read_token(); if (t != tIDENT) { db_cmd_list(cmd_table, aux_cmd_tablep, aux_cmd_tablep_end); db_flush_lex(); return; } } } if ((cmd->flag & CS_OWN) == 0) { /* * Standard syntax: * command [/modifier] [addr] [,count] @@ -360,30 +374,138 @@ db_command(last_cmdp, cmd_table, aux_cmd } else { db_dot = db_next; } } else { /* * If command does not change dot, * set 'next' location to be the same. */ db_next = db_dot; } } } + +int +db_cmd_add(struct command **set, struct command **set_end, boolean_t is_show) +{ + struct command **newset, **newset_end; + struct command ***oldset, ***oldset_end; + struct command **cpp, **xpp; + int *is_mallocd; + size_t old_count, new_count, holes; + + if (is_show) { + oldset = &db_show_cmd_start; + oldset_end = &db_show_cmd_end; + is_mallocd = &db_show_cmd_is_mallocd; + } else { + oldset = &db_cmd_start; + oldset_end = &db_cmd_end; + is_mallocd = &db_cmd_is_mallocd; + } + + new_count = set_end - set; + old_count = *oldset_end - *oldset; + holes = 0; + + for (cpp = *oldset; cpp < *oldset_end; cpp++) { + if (*cpp == NULL) + holes++; + } + + if (new_count > holes) { + size_t count; + + count = old_count - holes + new_count; + newset = malloc(count * sizeof(*cpp), M_TEMP, M_NOWAIT); + if (newset == NULL) + return ENOMEM; + newset_end = newset + count; + } else { + newset = *oldset; + newset_end = *oldset_end; + } + + xpp = newset; + for (cpp = *oldset; cpp < *oldset_end; cpp++) { + if (*cpp != NULL) + *xpp++ = *cpp; + } + for (cpp = set; cpp < set_end; cpp++) + *xpp++ = *cpp; + while (xpp < newset_end) + *xpp++ = NULL; + + cpp = *oldset; + + /* + * Using the debugger over these two assignments would be a + * bad idea. + */ + *oldset = newset; + *oldset_end = newset_end; + + if (new_count > holes) { + if (*is_mallocd) + free(cpp, M_TEMP); + *is_mallocd = 1; + } + + return 0; +} + +void +db_cmd_rem(struct command **set, struct command **set_end, boolean_t is_show) +{ + struct command **oldset, **oldset_end; + struct command **cpp, **xpp; + size_t new_count, holes; + + new_count = set_end - set; + if (is_show) { + oldset = db_show_cmd_start; + oldset_end = db_show_cmd_end; + } else { + oldset = db_cmd_start; + oldset_end = db_cmd_end; + } + + xpp = oldset; + holes = 0; + for (cpp = set; cpp < set_end; cpp++) { + /* + * Find *cpp. Items were copied into the global set + * in order, so they should be found in order. + */ + for (; xpp < oldset_end; xpp++) { + if (*xpp == *cpp) { + *xpp = NULL; + holes++; + break; + } + } + } + + if (holes != new_count) + printf("%s found %zd %scmd's; there should be %zd.\n", + __func__, holes, is_show ? "show_" : "", new_count); +} + + /* * 'show' commands */ static struct command db_show_all_cmds[] = { { "procs", db_ps, 0, 0 }, { (char *)0 } }; static struct command db_show_cmds[] = { { "all", 0, 0, db_show_all_cmds }, { "registers", db_show_regs, 0, 0 }, { "breaks", db_listbreak_cmd, 0, 0 }, { "threads", db_show_threads, 0, 0 }, { (char *)0, } @@ -444,31 +566,31 @@ db_command_loop() /* * Initialize 'prev' and 'next' to dot. */ db_prev = db_dot; db_next = db_dot; db_cmd_loop_done = 0; while (!db_cmd_loop_done) { if (db_print_position() != 0) db_printf("\n"); db_printf("db> "); (void) db_read_line(); db_command(&db_last_command, db_command_table, - SET_BEGIN(db_cmd_set), SET_LIMIT(db_cmd_set)); + db_cmd_start, db_cmd_end); } } void db_error(s) const char *s; { if (s) db_printf("%s", s); db_flush_lex(); kdb_reenter(); } /*
_______________________________________________ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"