lib/, src/: snprintf(3) already terminates strings with NUL

We don't need to terminate them manually after the call.  Remove all
that paranoid code, which in some cases was even wrong.  While at it,
let's do a few more things:

-  Use sizeof(buf) for the size of the buffer.  I found that a few cases
   were passing one less byte (probably because the last one was
   manually zeroed later).  This caused a double NUL.  snprintf(3) wants
   the size of the entire buffer to properly terminate it.  Passing the
   exact value hardcoded is brittle, so use sizeof().

-  Align and improve style of variable declarations.  This makes them
   appear in this diff, which will help review the patch.

Signed-off-by: Alejandro Colomar <alx@kernel.org>

diff --git a/lib/shell.c b/lib/shell.c
index b0d7e87d0be59f687ba94fde5b0495fde6c66720..b6f00f47127777615148d7138076cfd79a8f72ef 100644 (file)
--- a/lib/shell.c
+++ b/lib/shell.c
@@ -30,8 +30,8 @@ extern size_t newenvc;
 
 int shell (const char *file, /*@null@*/const char *arg, char *const envp[])
 {
-       char arg0[1024];
-       int err;
+       int   err;
+       char  arg0[1024];
 
        if (file == NULL) {
                errno = EINVAL;
@@ -45,8 +45,7 @@ int shell (const char *file, /*@null@*/const char *arg, char *const envp[])
         * don't want to tell us what it is themselves.
         */
        if (arg == NULL) {
-               (void) snprintf (arg0, sizeof arg0, "-%s", Basename (file));
-               arg0[sizeof arg0 - 1] = '\0';
+               snprintf(arg0, sizeof(arg0), "-%s", Basename(file));
                arg = arg0;
        }
 

diff --git a/lib/user_busy.c b/lib/user_busy.c
index eef645636bee8cf93d63c8bb72caf5a6a7579cd1..43d61984537bf4965c423e6999905592c3eafbff 100644 (file)
--- a/lib/user_busy.c
+++ b/lib/user_busy.c
@@ -151,16 +151,16 @@ static int check_status (const char *name, const char *sname, uid_t uid)
 
 static int user_busy_processes (const char *name, uid_t uid)
 {
-       DIR *proc;
-       struct dirent *ent;
-       char *tmp_d_name;
-       pid_t pid;
-       DIR *task_dir;
+       DIR            *proc;
+       DIR            *task_dir;
+       char           *tmp_d_name;
        /* 22: /proc/xxxxxxxxxx/task + \0 */
-       char task_path[22];
-       char root_path[22];
-       struct stat sbroot;
-       struct stat sbroot_process;
+       char           task_path[22];
+       char           root_path[22];
+       pid_t          pid;
+       struct stat    sbroot;
+       struct stat    sbroot_process;
+       struct dirent  *ent;
 
 #ifdef ENABLE_SUBIDS
        sub_uid_open (O_RDONLY);
@@ -205,8 +205,7 @@ static int user_busy_processes (const char *name, uid_t uid)
                }
 
                /* Check if the process is in our chroot */
-               snprintf (root_path, 22, "/proc/%lu/root", (unsigned long) pid);
-               root_path[21] = '\0';
+               snprintf(root_path, sizeof(root_path), "/proc/%lu/root", (unsigned long) pid);
                if (stat (root_path, &sbroot_process) != 0) {
                        continue;
                }
@@ -226,8 +225,7 @@ static int user_busy_processes (const char *name, uid_t uid)
                        return 1;
                }
 
-               snprintf (task_path, 22, "/proc/%lu/task", (unsigned long) pid);
-               task_path[21] = '\0';
+               snprintf(task_path, sizeof(task_path), "/proc/%lu/task", (unsigned long) pid);
                task_dir = opendir (task_path);
                if (task_dir != NULL) {
                        while ((ent = readdir (task_dir)) != NULL) {

diff --git a/src/gpasswd.c b/src/gpasswd.c
index 3936b85e6bbab18a26d9c1bff41e1397444aa408..390b04f92e0e710dbdb5a48a7c15cf9b9498564a 100644 (file)
--- a/src/gpasswd.c
+++ b/src/gpasswd.c
@@ -383,17 +383,16 @@ static void open_files (void)
 static void log_gpasswd_failure (const char *suffix)
 {
 #ifdef WITH_AUDIT
-       char buf[1024];
+       char  buf[1024];
 #endif
        if (aflg) {
                SYSLOG ((LOG_ERR,
                         "%s failed to add user %s to group %s%s",
                         myname, user, group, suffix));
 #ifdef WITH_AUDIT
-               snprintf (buf, 1023,
-                         "%s failed to add user %s to group %s%s",
-                         myname, user, group, suffix);
-               buf[1023] = '\0';
+               snprintf(buf, sizeof(buf),
+                        "%s failed to add user %s to group %s%s",
+                        myname, user, group, suffix);
                audit_logger (AUDIT_USER_ACCT, Prog,
                              buf,
                              group, AUDIT_NO_ID,
@@ -404,10 +403,9 @@ static void log_gpasswd_failure (const char *suffix)
                         "%s failed to remove user %s from group %s%s",
                         myname, user, group, suffix));
 #ifdef WITH_AUDIT
-               snprintf (buf, 1023,
-                         "%s failed to remove user %s from group %s%s",
-                         myname, user, group, suffix);
-               buf[1023] = '\0';
+               snprintf(buf, sizeof(buf),
+                        "%s failed to remove user %s from group %s%s",
+                        myname, user, group, suffix);
                audit_logger (AUDIT_USER_ACCT, Prog,
                              buf,
                              group, AUDIT_NO_ID,
@@ -418,10 +416,9 @@ static void log_gpasswd_failure (const char *suffix)
                         "%s failed to remove password of group %s%s",
                         myname, group, suffix));
 #ifdef WITH_AUDIT
-               snprintf (buf, 1023,
-                         "%s failed to remove password of group %s%s",
-                         myname, group, suffix);
-               buf[1023] = '\0';
+               snprintf(buf, sizeof(buf),
+                        "%s failed to remove password of group %s%s",
+                        myname, group, suffix);
                audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
                              buf,
                              group, AUDIT_NO_ID,
@@ -432,10 +429,9 @@ static void log_gpasswd_failure (const char *suffix)
                         "%s failed to restrict access to group %s%s",
                         myname, group, suffix));
 #ifdef WITH_AUDIT
-               snprintf (buf, 1023,
-                         "%s failed to restrict access to group %s%s",
-                         myname, group, suffix);
-               buf[1023] = '\0';
+               snprintf(buf, sizeof(buf),
+                        "%s failed to restrict access to group %s%s",
+                        myname, group, suffix);
                audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
                              buf,
                              group, AUDIT_NO_ID,
@@ -448,10 +444,9 @@ static void log_gpasswd_failure (const char *suffix)
                                 "%s failed to set the administrators of group %s to %s%s",
                                 myname, group, admins, suffix));
 #ifdef WITH_AUDIT
-                       snprintf (buf, 1023,
-                                 "%s failed to set the administrators of group %s to %s%s",
-                                 myname, group, admins, suffix);
-                       buf[1023] = '\0';
+                       snprintf(buf, sizeof(buf),
+                                "%s failed to set the administrators of group %s to %s%s",
+                                myname, group, admins, suffix);
                        audit_logger (AUDIT_USER_ACCT, Prog,
                                      buf,
                                      group, AUDIT_NO_ID,
@@ -464,10 +459,9 @@ static void log_gpasswd_failure (const char *suffix)
                                 "%s failed to set the members of group %s to %s%s",
                                 myname, group, members, suffix));
 #ifdef WITH_AUDIT
-                       snprintf (buf, 1023,
-                                 "%s failed to set the members of group %s to %s%s",
-                                 myname, group, members, suffix);
-                       buf[1023] = '\0';
+                       snprintf(buf, sizeof(buf),
+                                "%s failed to set the members of group %s to %s%s",
+                                myname, group, members, suffix);
                        audit_logger (AUDIT_USER_ACCT, Prog,
                                      buf,
                                      group, AUDIT_NO_ID,
@@ -479,10 +473,9 @@ static void log_gpasswd_failure (const char *suffix)
                         "%s failed to change password of group %s%s",
                         myname, group, suffix));
 #ifdef WITH_AUDIT
-               snprintf (buf, 1023,
-                         "%s failed to change password of group %s%s",
-                         myname, group, suffix);
-               buf[1023] = '\0';
+               snprintf(buf, sizeof(buf),
+                        "%s failed to change password of group %s%s",
+                        myname, group, suffix);
                audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
                              buf,
                              group, AUDIT_NO_ID,
@@ -498,18 +491,18 @@ static void log_gpasswd_failure_system (unused void *arg)
 
 static void log_gpasswd_failure_group (unused void *arg)
 {
-       char buf[1024];
-       snprintf (buf, 1023, " in %s", gr_dbname ());
-       buf[1023] = '\0';
+       char  buf[1024];
+
+       snprintf(buf, sizeof(buf), " in %s", gr_dbname());
        log_gpasswd_failure (buf);
 }
 
 #ifdef SHADOWGRP
 static void log_gpasswd_failure_gshadow (unused void *arg)
 {
-       char buf[1024];
-       snprintf (buf, 1023, " in %s", sgr_dbname ());
-       buf[1023] = '\0';
+       char  buf[1024];
+
+       snprintf(buf, sizeof(buf), " in %s", sgr_dbname());
        log_gpasswd_failure (buf);
 }
 #endif                         /* SHADOWGRP */
@@ -517,17 +510,16 @@ static void log_gpasswd_failure_gshadow (unused void *arg)
 static void log_gpasswd_success (const char *suffix)
 {
 #ifdef WITH_AUDIT
-       char buf[1024];
+       char  buf[1024];
 #endif
        if (aflg) {
                SYSLOG ((LOG_INFO,
                         "user %s added by %s to group %s%s",
                         user, myname, group, suffix));
 #ifdef WITH_AUDIT
-               snprintf (buf, 1023,
-                         "user %s added by %s to group %s%s",
-                         user, myname, group, suffix);
-               buf[1023] = '\0';
+               snprintf(buf, sizeof(buf),
+                        "user %s added by %s to group %s%s",
+                        user, myname, group, suffix);
                audit_logger (AUDIT_USER_ACCT, Prog,
                              buf,
                              group, AUDIT_NO_ID,
@@ -538,10 +530,9 @@ static void log_gpasswd_success (const char *suffix)
                         "user %s removed by %s from group %s%s",
                         user, myname, group, suffix));
 #ifdef WITH_AUDIT
-               snprintf (buf, 1023,
-                         "user %s removed by %s from group %s%s",
-                         user, myname, group, suffix);
-               buf[1023] = '\0';
+               snprintf(buf, sizeof(buf),
+                        "user %s removed by %s from group %s%s",
+                        user, myname, group, suffix);
                audit_logger (AUDIT_USER_ACCT, Prog,
                              buf,
                              group, AUDIT_NO_ID,
@@ -552,10 +543,9 @@ static void log_gpasswd_success (const char *suffix)
                         "password of group %s removed by %s%s",
                         group, myname, suffix));
 #ifdef WITH_AUDIT
-               snprintf (buf, 1023,
-                         "password of group %s removed by %s%s",
-                         group, myname, suffix);
-               buf[1023] = '\0';
+               snprintf(buf, sizeof(buf),
+                        "password of group %s removed by %s%s",
+                        group, myname, suffix);
                audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
                              buf,
                              group, AUDIT_NO_ID,
@@ -566,10 +556,9 @@ static void log_gpasswd_success (const char *suffix)
                         "access to group %s restricted by %s%s",
                         group, myname, suffix));
 #ifdef WITH_AUDIT
-               snprintf (buf, 1023,
-                         "access to group %s restricted by %s%s",
-                         group, myname, suffix);
-               buf[1023] = '\0';
+               snprintf(buf, sizeof(buf),
+                        "access to group %s restricted by %s%s",
+                        group, myname, suffix);
                audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
                              buf,
                              group, AUDIT_NO_ID,
@@ -582,10 +571,9 @@ static void log_gpasswd_success (const char *suffix)
                                 "administrators of group %s set by %s to %s%s",
                                 group, myname, admins, suffix));
 #ifdef WITH_AUDIT
-                       snprintf (buf, 1023,
-                                 "administrators of group %s set by %s to %s%s",
-                                 group, myname, admins, suffix);
-                       buf[1023] = '\0';
+                       snprintf(buf, sizeof(buf),
+                                "administrators of group %s set by %s to %s%s",
+                                group, myname, admins, suffix);
                        audit_logger (AUDIT_USER_ACCT, Prog,
                                      buf,
                                      group, AUDIT_NO_ID,
@@ -598,10 +586,9 @@ static void log_gpasswd_success (const char *suffix)
                                 "members of group %s set by %s to %s%s",
                                 group, myname, members, suffix));
 #ifdef WITH_AUDIT
-                       snprintf (buf, 1023,
-                                 "members of group %s set by %s to %s%s",
-                                 group, myname, members, suffix);
-                       buf[1023] = '\0';
+                       snprintf(buf, sizeof(buf),
+                                "members of group %s set by %s to %s%s",
+                                group, myname, members, suffix);
                        audit_logger (AUDIT_USER_ACCT, Prog,
                                      buf,
                                      group, AUDIT_NO_ID,
@@ -613,10 +600,9 @@ static void log_gpasswd_success (const char *suffix)
                         "password of group %s changed by %s%s",
                         group, myname, suffix));
 #ifdef WITH_AUDIT
-               snprintf (buf, 1023,
-                         "password of group %s changed by %s%s",
-                         group, myname, suffix);
-               buf[1023] = '\0';
+               snprintf(buf, sizeof(buf),
+                        "password of group %s changed by %s%s",
+                        group, myname, suffix);
                audit_logger (AUDIT_USER_CHAUTHTOK, Prog,
                              buf,
                              group, AUDIT_NO_ID,
@@ -632,9 +618,9 @@ static void log_gpasswd_success_system (unused void *arg)
 
 static void log_gpasswd_success_group (unused void *arg)
 {
-       char buf[1024];
-       snprintf (buf, 1023, " in %s", gr_dbname ());
-       buf[1023] = '\0';
+       char  buf[1024];
+
+       snprintf(buf, sizeof(buf), " in %s", gr_dbname());
        log_gpasswd_success (buf);
 }