Error injection

Richard Sharpe realrichardsharpe at gmail.com
Wed Mar 25 19:59:39 GMT 2009


Hi,

I have added Error Injection to the Samba 3.0.31 base that is used at
Data Domain because there was a need for it.

Here follows the patches between the before-error-injection code and
the after-error-injection code so that people can see what it
involves. I hope that this spawns some discussion on the subject of
error injection in Samba in general, because it is an important tool
in ensuring good code path coverage in the case of error paths that
are hard to provoke during testing. It is also a widely used
technique. I have implemented it in C++ before but this time around I
have used features from modern C compilers. Configure would have to
have tests to ensure that C constructors are supported to allow the
code to be enabled.

The primary notion is that each source file is a module and an error
injection table will be produced for each module in which certain
macros are used. There are some examples of using error injection in a
couple of modules as well (maybe only one).

I do not claim that this is the best way to implement error injection,
and we will probably switch to whatever the Samba team settles on if
something is decided upon.

There is still one deficiency in what I have implemented, and that is
it should be possible to switch on error injection on a program by
program basis (ie, for smbd only, say, rather than for all programs
that use a particular module/file). That will be my next internal
change.

The way in which error injection is enabled/disabled is conducive to
being hooked into the smbcontrol framework that James mentioned a few
days ago.

error_injection.h
#if defined(DD) && defined(DEVELOPER)
/*
 * Error injection framework. We keep a list of the error injection modules
 * They are managed through macros that create GCC constructors in each file
 * that has error injection points. Each of the constructors call this
 * registration routine, which copies the error injection table it is passed
 * because the passed in table is created on the stack. We hand back a handle
 * to the table and do some setup.
 * We keep this in an include file so we can include in several programs.
 */
static int dd_error_injection_table_count = 0;
static dd_error_injection_table_t *dd_error_injection_table = NULL;

dd_error_injection_table_t *dd_register_error_points(const char *name,
                        void *error_tab)
{
        int err_table_entries = 0, i = 0;
        dd_error_injection_entry_t *error_injection_entry;
        dd_error_injection_t *error_table = (dd_error_injection_t *)error_tab;

        /* Figure out how many entries there are in the table */
        while (error_table->name != NULL &&
                error_table->injection_point != 0) {
                err_table_entries++;
                error_table++;
        }

        /* Reset this variable */
        error_table = (dd_error_injection_t *)error_tab;

        /* Now, allocate space for the table */
        error_injection_entry =
                (dd_error_injection_entry_t
*)SMB_MALLOC(sizeof(dd_error_injection_entry_t) * err_table_entries);
        if (!error_injection_entry) { /* some bad error occurred */
                printf("Unable to allocate space to register error injection "
                        "table. Bailing, sorry: %s\n", strerror(errno));
                _exit(1);
        }

        /* Now initialize the table by copying the table */
        for (i = 0; i < err_table_entries; i++) {
                error_injection_entry[i].calls = 0;
                error_injection_entry[i].entry.injection_point =
                        error_table[i].injection_point;
                error_injection_entry[i].entry.enabled         = False;
                error_injection_entry[i].entry.type            =
                        error_table[i].type;
                error_injection_entry[i].entry.count           = 0;
                /* This is a literal string, so copy ptr only */
                error_injection_entry[i].entry.name            =
                        error_table[i].name;
        }

        /* Now, realloc the master table and insert this as the next entry */
        dd_error_injection_table = SMB_REALLOC_ARRAY(dd_error_injection_table,
                                        dd_error_injection_table_t,
                                        ++dd_error_injection_table_count);
        if (!dd_error_injection_table) {
                printf("Unable to reallocate error injection table. Bailing!"
                        " %s\n", strerror(errno));
                _exit(2);
        }

        /* Add the new entry to the table */
        dd_error_injection_table[dd_error_injection_table_count -
1].entry_count = err_table_entries;
        dd_error_injection_table[dd_error_injection_table_count - 1].name =
                name;
        dd_error_injection_table[dd_error_injection_table_count - 1].table =
                error_injection_entry;

        printf("Adding %d entries to error injection table %s\n",
err_table_entries, name);

        /* Return the handle */
        return &dd_error_injection_table[dd_error_injection_table_count - 1];
}

/*
 * We take the table handle and the index and check if the error should fire.
 */
int dd_check_error_enabled(dd_error_injection_table_t *handle,
unsigned int num){
        dd_error_injection_table_t *err_table = handle;
        printf("Error injection for module %s number %u called\n",
                err_table->name, num);

        if (num > err_table->entry_count)
                return False;  /* Not here buddy */

        err_table->table[num - 1].calls++;

        if (!err_table->table[num - 1].entry.enabled)
                return False;

        if (err_table->table[num - 1].entry.type == DD_EVERY &&
                (err_table->table[num - 1].calls %
                        err_table->table[num - 1].entry.count) == 0)
                return True;

        if (err_table->table[num - 1].entry.type == DD_UNTIL &&
                (err_table->table[num - 1].calls <=
                        err_table->table[num - 1].entry.count))
                return True;

        if (err_table->table[num - 1].entry.type == DD_AFTER &&
                (err_table->table[num - 1].calls >
                        err_table->table[num - 1].entry.count))
                return True;

        return False; /* Did not fire */
}

/*
 * Initialize the Error injection system. We retrieve the error injection
 * enable from the smb.conf, process it and enable all errors that are
 * specified in the list.
 *
 * It provides a simple parser that parses strings like:
 *
 * "enable|disable module-name.inj-point-num every|until|after <count>
 */

enum dd_error_command_t {DD_ENABLE = 0, DD_DISABLE};

static void dd_parse_and_set_injection_points(const char * inj_string)
{
        pstring *tok_tmp, tok, module, inj_point_str;
        unsigned int inj_point, i = 0, count;
        const char *inj_string_tmp = inj_string;
        enum dd_error_command_t cmd;
        dd_error_injection_table_t *moduleptr = NULL;
        enum dd_injection_type_t injection_type;

        DEBUG(10, ("Parsing error injection string: %s\n", inj_string));

        if (!next_token_nr(&inj_string_tmp, tok, NULL, sizeof(tok))) {
                DEBUG(0, ("Unable to parse injection enable string: %s\n",
                        inj_string));
                return;
        }
        /* Parse the command: enable|disable */
        if (!strncmp(tok, "enable", 6)) {
                cmd = DD_ENABLE;
        } else if (!strncmp(tok, "disable", 7)) {
                cmd = DD_DISABLE;
        } else {
                DEBUG(0, ("Unknown injection command: %s %s\n", tok,
                        inj_string));
                return;
        }

        /* parse out the module and injection point number */
        if (!next_token_nr(&inj_string_tmp, tok, NULL, sizeof(tok))) {
                DEBUG(0, ("Unable to parse module name from string: %s\n",
                        inj_string));
                return;
        }

        tok_tmp = &tok;
        /* It consists of module_name.injection-point-num */
        if (!next_token(&tok_tmp, module, ".", sizeof(module))) {
                DEBUG(0, ("Invalid syntax for modulename and injection point: "
                        "%s\n", tok));
                return;
        }

        if (!next_token(&tok_tmp, inj_point_str, NULL,
sizeof(inj_point_str))) {                DEBUG(0, ("Invalid syntax for
modulename and injection point: "
                        "%s\n", tok));
                return;
        }

        DEBUG(10, ("Module: %s, injection point: %s\n", module, inj_point_str));
        /* Search for the module. If not found, bail. No more parsing */

        for (i = 0; i < dd_error_injection_table_count; i++) {
                if (!strcmp(dd_error_injection_table[i].name,
                        module))
                        moduleptr = &dd_error_injection_table[i];
        }

        if (!moduleptr) {
                DEBUG(0, ("Unknown error injection module: %s\n", module));
                return;
        }

        /* Is the injection point valid? */
        inj_point = atoi(inj_point_str);
        if (inj_point == 0 || inj_point > moduleptr->entry_count) {
                DEBUG(0, ("Injection point invalid (0, too large, or not a "
                        "number: %s\n", inj_point_str));
                DEBUG(0, ("Module entry count: %d %d\n", inj_point,
                        moduleptr->entry_count));
                return;
        }

        DEBUG(10, ("Injection point valid\n"));
        /* Now parse out the type of arming and the count */
        if (!next_token_nr(&inj_string_tmp, tok, NULL, sizeof(tok))) {
                DEBUG(0, ("Unable to parse injection type from string: %s\n",
                        inj_string));
                return;
        }
        if (!strncmp(tok, "every", 5))
                injection_type = DD_EVERY;
        else if (!strncmp(tok, "until", 5))
                injection_type = DD_UNTIL;
        else if (!strncmp(tok, "after", 5))
                injection_type = DD_AFTER;
        else {
                DEBUG(0, ("Invalid injection type %s in string %s!\n", tok,
                        inj_string));
                return;
        }

        DEBUG(10, ("Injection type OK\n"));
        if (!next_token_nr(&inj_string_tmp, tok, NULL, sizeof(tok))) {
                DEBUG(0, ("Unable to parse injection count from string: %s\n",
                        inj_string));
                return;
        }

        count = atoi(tok);

        if (count == 0 && cmd == DD_ENABLE) {
                DEBUG(0, ("Invalid combination enable and count of 0 in %s\n",
                        inj_string));
                return;
        }

        DEBUG(10, ("Injection count OK\n"));

        moduleptr->table[inj_point - 1].entry.enabled = cmd == DD_ENABLE;
        moduleptr->table[inj_point - 1].entry.type    = injection_type;
        moduleptr->table[inj_point - 1].entry.count   = count;
        /* Make sure we start from zero when enabled */
        moduleptr->table[inj_point - 1].calls         = 0;

        DEBUG(0, ("Processed error injection string: %s\n", inj_string));
}

static void dd_error_injection_init()
{
        char *error_inj_string = lp_dd_error_injection();
        char *error_inj_tmp = error_inj_string;
        pstring one_error_str;

        if (!error_inj_string || !error_inj_string[0])
                return;   /* Nothing to parse ... */

        /*
         * split the string up on semicolons and feed them into
         * dd_parse_and_set_injection_points
         */

        while (next_token_nr(&error_inj_tmp, one_error_str, ";",
                        sizeof(one_error_str))) {
                dd_parse_and_set_injection_points(one_error_str);
        }
}

#endif

==== //prod/main/platform/user/samba/source/client/client.c#3 -
/data/prod/main/platform/user/samba/source/client/client.c ====
--- /tmp/tmp.5567.0     2009-03-25 12:36:23.000000000 -0700
+++ /data/prod/main/platform/user/samba/source/client/client.c
2009-03-24 14:29:12.000000000 -0700
@@ -28,6 +28,8 @@
 #define REGISTER 0
 #endif

+#include "error_injection.h"
+
 extern BOOL AllowDebugChange;
 extern BOOL override_logfile;
 extern char tar_type;
@@ -4047,6 +4049,10 @@ static int do_message_op(void)

        load_interfaces();

+#if defined(DD) && defined(DEVELOPER)
+       dd_error_injection_init();
+#endif
+
        if (service_opt) {
                /* Convert any '/' characters in the service name to
'\' characters */
                string_replace(service, '/','\\');
==== //prod/main/platform/user/samba/source/client/smbspool.c#2 -
/data/prod/main/platform/user/samba/source/client/smbspool.c ====
--- /tmp/tmp.5567.1     2009-03-25 12:36:23.000000000 -0700
+++ /data/prod/main/platform/user/samba/source/client/smbspool.c
 2009-03-24 14:29:12.000000000 -0700
@@ -31,6 +31,7 @@
 #define KRB5CCNAME               "KRB5CCNAME"
 #define MAX_RETRY_CONNECT        3

+#include "error_injection.h"

 /*
  * Globals...
@@ -131,6 +132,10 @@ static char *              uri_unescape_alloc(const
   else
     copies = atoi(argv[4]);

+#if defined(DD) && defined(DEVELOPER)
+       dd_error_injection_init();
+#endif
+
  /*
   * Find the URI...
   */
==== //prod/main/platform/user/samba/source/include/debug.h#2 -
/data/prod/main/platform/user/samba/source/include/debug.h ====
--- /tmp/tmp.5567.2     2009-03-25 12:36:23.000000000 -0700
+++ /data/prod/main/platform/user/samba/source/include/debug.h
2009-03-24 16:39:28.000000000 -0700
@@ -103,6 +103,7 @@ extern int DEBUGLEVEL;
 #define DBGC_LOCKING           16
 #define DBGC_MSDFS             17
 #define DBGC_DMAPI             18
+#define DBGC_UTIL_EVENTLOG     19

 /* So you can define DBGC_CLASS before including debug.h */
 #ifndef DBGC_CLASS
==== //prod/main/platform/user/samba/source/include/includes.h#10 -
/data/prod/main/platform/user/samba/source/include/includes.h ====
--- /tmp/tmp.5567.3     2009-03-25 12:36:23.000000000 -0700
+++ /data/prod/main/platform/user/samba/source/include/includes.h
 2009-03-24 14:29:12.000000000 -0700
@@ -904,6 +904,80 @@ enum flush_reason_enum {
 #include "nss_info.h"
 #include "modules/nfs4_acls.h"
+/* Need this before proto.h include */
+#if defined(DD) && defined(DEVELOPER)
+/* Define and populate error injection tables. This generates a gcc
constructor+ * that adds the tables and their entries before main is
called. It also
+ * populates a static variable with a handle for the table
+ */
+enum dd_injection_type_t {DD_EVERY = 0, DD_AFTER, DD_UNTIL};
+typedef struct {
+       int injection_point;  /* Redundant? Is the table number */
+       int enabled;          /* Is this enabled?               */
+       enum dd_injection_type_t type;
+       int count;
+       char *name;
+} dd_error_injection_t;
+
+typedef struct {
+       int calls;  /* How many times we have been this way */
+       dd_error_injection_t entry;
+} dd_error_injection_entry_t;
+
+typedef struct {
+       unsigned int entry_count;
+       unsigned char *name;
+       dd_error_injection_entry_t *table;
+} dd_error_injection_table_t;
+
+/*
+ * This function copies all data passed in, since it is declared on the
+ * stack typically.
+ */
+dd_error_injection_table_t *dd_register_error_points(const char *name,
+                               void *error_table);
+
+int dd_check_error_enabled(dd_error_injection_table_t *thandle,
unsigned int num);
+
+#define DD_ERROR_TABLE_START(_name_) \
+static dd_error_injection_table_t *dd_error_injection_table = NULL; \
+void dd_##_name_##_error_injection_table_register(void) \
+       __attribute__ ((constructor)); \
+\
+void dd_##_name_##_error_injection_table_register(void) \
+{ \
+       char *module_name = #_name_; \
+       dd_error_injection_t error_table[] = {
+
+#define DD_ERROR_TABLE_ENTRY(_num_, _name_) \
+               { _num_, 0, DD_EVERY, 0, _name_ },
+
+#define DD_ERROR_TABLE_END \
+               { 0, 0, 0, 0, 0 } \
+       }; \
+       dd_error_injection_table = dd_register_error_points(module_name, \
+                                       (void *)error_table); \
+       if (!dd_error_injection_table) { \
+               printf("Unable to register error injection table: %s\n", \
+                       module_name); \
+               _exit(1); \
+       } \
+}
+
+#define DD_ERROR_INJECTION(_comment_, _num_, _action_) \
+       if (dd_check_error_enabled(dd_error_injection_table, _num_)) { \
+               DEBUG(0, ("Error injection %d in module __FILE__
fired: %s\n", \+                       _num_, _comment_)); \
+               _action_; \
+       }
+
+#else
+#define DD_ERROR_TABLE_START(_name_)
+#define DD_ERROR_TABLE_ENTRY(_num_, _name_)
+#define DD_ERROR_TABLE_END
+#define DD_ERROR_INJECTION(_comment_, _num_, _action_)
+#endif
+
 /***** automatically generated prototypes *****/
 #ifndef NO_PROTO_H
 #include "proto.h"
==== //prod/main/platform/user/samba/source/lib/util_tdb.c#3 -
/data/prod/main/platform/user/samba/source/lib/util_tdb.c ====
--- /tmp/tmp.5567.4     2009-03-25 12:36:23.000000000 -0700
+++ /data/prod/main/platform/user/samba/source/lib/util_tdb.c
2009-03-24 14:29:12.000000000 -0700
@@ -25,6 +25,14 @@
 #undef calloc
 #undef strdup

+/*
+ * DD Error injection
+ */
+
+DD_ERROR_TABLE_START(UTIL_TDB)
+DD_ERROR_TABLE_ENTRY(1, "DD_PRETEND_TIMEOUT")
+DD_ERROR_TABLE_END
+
 /* these are little tdb utility functions that are meant to make
    dealing with a tdb database a little less cumbersome in Samba */

@@ -77,6 +85,9 @@ static int tdb_chainlock_with_timeout_in
        else
                ret = tdb_chainlock(tdb, key);

+       DD_ERROR_INJECTION("Pretend we got a timeout", 1,
+               (gotalarm = 1));
+
        if (timeout) {
                alarm(0);
                tdb_setalarm_sigptr(tdb, NULL);
==== //prod/main/platform/user/samba/source/nmbd/nmbd.c#2 -
/data/prod/main/platform/user/samba/source/nmbd/nmbd.c ====
--- /tmp/tmp.5567.5     2009-03-25 12:36:23.000000000 -0700
+++ /data/prod/main/platform/user/samba/source/nmbd/nmbd.c
2009-03-24 14:29:12.000000000 -0700
@@ -49,6 +49,8 @@ BOOL found_lm_clients = False;

 time_t StartupTime = 0;

+#include "error_injection.h"
+
 /****************************************************************************
**
  Handle a SIGTERM in band.
  ****************************************************************************
*/
@@ -703,6 +705,10 @@ static BOOL open_sockets(BOOL isdaemon,
                slprintf(logfile, sizeof(logfile)-1, "%s/log.nmbd",
dyn_LOGFILEBASE);
                lp_set_logfile(logfile);
        }
+
+#if defined(DD) && defined(DEVELOPER)
+       dd_error_injection_init();
+#endif

        fault_setup((void (*)(void *))fault_continue );
        dump_core_setup("nmbd");
==== //prod/main/platform/user/samba/source/nsswitch/wbinfo.c#2 -
/data/prod/main/platform/user/samba/source/nsswitch/wbinfo.c ====
--- /tmp/tmp.5567.6     2009-03-25 12:36:23.000000000 -0700
+++ /data/prod/main/platform/user/samba/source/nsswitch/wbinfo.c
 2009-03-24 14:29:12.000000000 -0700
@@ -30,6 +30,8 @@

 extern int winbindd_fd;

+#include "error_injection.h"
+
 static char winbind_separator_int(BOOL strict)
 {
        struct winbindd_response response;
@@ -1272,6 +1274,10 @@ int main(int argc, char **argv, char **e
                exit(1);
        }

+#if defined(DD) && defined(DEVELOPER)
+       dd_error_injection_init();
+#endif
+
        if (!init_names())
                return 1;

==== //prod/main/platform/user/samba/source/nsswitch/winbindd.c#2 -
/data/prod/main/platform/user/samba/source/nsswitch/winbindd.c ====
--- /tmp/tmp.5567.7     2009-03-25 12:36:23.000000000 -0700
+++ /data/prod/main/platform/user/samba/source/nsswitch/winbindd.c
 2009-03-24 14:29:12.000000000 -0700
@@ -29,6 +29,8 @@
 #undef DBGC_CLASS
 #define DBGC_CLASS DBGC_WINBIND

+#include "error_injection.h"
+
 BOOL opt_nocache = False;
 static BOOL interactive = False;

@@ -1012,6 +1014,10 @@ int main(int argc, char **argv, char **e
                exit(1);
        }

+#if defined(DD) && defined(DEVELOPER)
+       dd_error_injection_init();
+#endif
+
        if (!directory_exist(lp_lockdir(), NULL)) {
                mkdir(lp_lockdir(), 0755);
        }
==== //prod/main/platform/user/samba/source/param/loadparm.c#17 -
/data/prod/main/platform/user/samba/source/param/loadparm.c ====
--- /tmp/tmp.5567.8     2009-03-25 12:36:23.000000000 -0700
+++ /data/prod/main/platform/user/samba/source/param/loadparm.c
2009-03-24 14:29:12.000000000 -0700
@@ -344,6 +344,7 @@ typedef struct {
         BOOL bUseKerberosReplayCache;
         int ddGroupNodeRole;
         char *szDdClusterName;
+       char *szDdErrorInjection;
 #endif
 } global;

@@ -1379,6 +1380,7 @@ static struct parm_struct parm_table[] =
         {"dd use kerberos replay cache", P_BOOL, P_GLOBAL,
&Globals.bUseKerberosReplayCache, NULL, NULL, FLAG_HIDE},
         {"dd group node role", P_INTEGER, P_GLOBAL,
&Globals.ddGroupNodeRole, NULL, NULL, FLAG_ADVANCED},
         {"dd cluster name", P_STRING, P_GLOBAL,
&Globals.szDdClusterName, NULL, NULL, FLAG_ADVANCED},
+       {"dd error injection", P_STRING, P_GLOBAL, &Globals.szDdErrorInjection,
NULL, NULL, FLAG_ADVANCED},
 #endif
        {NULL,  P_BOOL,  P_NONE,  NULL,  NULL,  NULL,  0}
 };
@@ -1792,6 +1794,7 @@ static void init_globals(BOOL first_time
         Globals.bUseKerberosReplayCache = True;
         Globals.ddGroupNodeRole = 0; /* Officially: 0x0001 is the
value from dd_group.h DD_GROUP_NODE_ROLE_SINGLE but actually, they use
0 for single mode */
         string_set(&Globals.szDdClusterName, ""); /* dd cluster name
is not defined by default */
+       string_set(&Globals.szDdErrorInjection, ""); /* No Error injection */
 #endif
 }

@@ -2309,6 +2312,7 @@ FN_GLOBAL_LIST(lp_dd_ddns_interfaces, &G
 FN_GLOBAL_BOOL(lp_dd_use_kerberos_replay_cache,
&Globals.bUseKerberosReplayCache)
 FN_GLOBAL_INTEGER(lp_dd_group_node_role, &Globals.ddGroupNodeRole)
 FN_GLOBAL_STRING(lp_dd_cluster_name, &Globals.szDdClusterName)
+FN_GLOBAL_STRING(lp_dd_error_injection, &Globals.szDdErrorInjection)
 #endif

 /* local prototypes */
...
==== //prod/main/platform/user/samba/source/smbd/server.c#9 -
/data/prod/main/platform/user/samba/source/smbd/server.c ====
--- /tmp/tmp.5567.10    2009-03-25 12:36:23.000000000 -0700
+++ /data/prod/main/platform/user/samba/source/smbd/server.c
2009-03-24 14:29:12.000000000 -0700
@@ -46,6 +46,8 @@ extern int am_child_smbd;
 extern int dcelogin_atmost_once;
 #endif /* WITH_DFS */

+#include "error_injection.h"
+
 /* really we should have a top level context structure that has the
    client file descriptor as an element. That would require a major rewrite :(

@@ -1035,6 +1037,11 @@ extern void build_options(BOOL screen);

        DEBUG(3,( "loaded services\n"));

+#if defined(DD) && defined(DEVELOPER)
+       /* Init error injection */
+       dd_error_injection_init();
+#endif
+
        if (!is_daemon && !is_a_socket(0)) {
 #ifdef DD
                /*


-- 
Regards,
Richard Sharpe


More information about the samba-technical mailing list