From 3e4b3d385dfca1b2c64eadd793705dd4fae3b297 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Mon, 20 Jan 2025 19:39:12 +0100 Subject: [PATCH] lib/, src/: Turn error counters into flags If we are not interested in the amount of errors but only if errors exist, use a flag instead of a counter. This eliminates the chance of signed integer overflows and better reflects the meaning of variable. Keeping variable name and basically copied from src/faillog.c. Reviewed-by: Alejandro Colomar Signed-off-by: Tobias Stoeckmann --- lib/commonio.c | 28 ++++++++++++++-------------- src/chgpasswd.c | 14 +++++++------- src/chpasswd.c | 16 ++++++++-------- src/grpck.c | 40 ++++++++++++++++++++-------------------- src/pwck.c | 48 ++++++++++++++++++++++++------------------------ src/useradd.c | 6 +++--- src/userdel.c | 35 ++++++++++++++++++++--------------- src/usermod.c | 6 +++--- 8 files changed, 99 insertions(+), 94 deletions(-) diff --git a/lib/commonio.c b/lib/commonio.c index 4d83e83cb..b7c9a2d41 100644 --- a/lib/commonio.c +++ b/lib/commonio.c @@ -891,7 +891,7 @@ static int write_all (const struct commonio_db *db) int commonio_close (struct commonio_db *db) { - int errors = 0; + bool errors = false; char buf[1024]; struct stat sb; @@ -932,25 +932,25 @@ int commonio_close (struct commonio_db *db) #ifdef WITH_SELINUX if (set_selinux_file_context (db->filename, S_IFREG) != 0) { - errors++; + errors = true; } #endif if (create_backup (buf, db->fp) != 0) { - errors++; + errors = true; } if (fclose (db->fp) != 0) { - errors++; + errors = true; } db->fp = NULL; #ifdef WITH_SELINUX if (reset_selinux_file_context () != 0) { - errors++; + errors = true; } #endif - if (errors != 0) + if (errors) goto fail; } else { /* @@ -966,7 +966,7 @@ int commonio_close (struct commonio_db *db) #ifdef WITH_SELINUX if (set_selinux_file_context (db->filename, S_IFREG) != 0) { - errors++; + errors = true; } #endif @@ -976,24 +976,24 @@ int commonio_close (struct commonio_db *db) } if (write_all (db) != 0) { - errors++; + errors = true; } if (fflush (db->fp) != 0) { - errors++; + errors = true; } if (fsync (fileno (db->fp)) != 0) { - errors++; + errors = true; } if (fclose (db->fp) != 0) { - errors++; + errors = true; } db->fp = NULL; - if (errors != 0) { + if (errors) { unlink (buf); goto fail; } @@ -1011,11 +1011,11 @@ int commonio_close (struct commonio_db *db) nscd_need_reload = true; goto success; fail: - errors++; + errors = true; success: free_linked_list (db); - return errors == 0; + return !errors; } static /*@dependent@*/ /*@null@*/struct commonio_entry *next_entry_by_name ( diff --git a/src/chgpasswd.c b/src/chgpasswd.c index 807ef15d3..1625e8540 100644 --- a/src/chgpasswd.c +++ b/src/chgpasswd.c @@ -426,7 +426,7 @@ int main (int argc, char **argv) const struct group *gr; struct group newgr; - int errors = 0; + bool errors = false; intmax_t line = 0; log_set_progname(Prog); @@ -466,7 +466,7 @@ int main (int argc, char **argv) if (stpsep(buf, "\n") == NULL) { fprintf (stderr, _("%s: line %jd: line too long\n"), Prog, line); - errors++; + errors = true; continue; } @@ -485,7 +485,7 @@ int main (int argc, char **argv) fprintf (stderr, _("%s: line %jd: missing new password\n"), Prog, line); - errors++; + errors = true; continue; } newpwd = cp; @@ -536,7 +536,7 @@ int main (int argc, char **argv) fprintf (stderr, _("%s: line %jd: group '%s' does not exist\n"), Prog, line, name); - errors++; + errors = true; continue; } #ifdef SHADOWGRP @@ -596,7 +596,7 @@ int main (int argc, char **argv) fprintf (stderr, _("%s: line %jd: failed to prepare the new %s entry '%s'\n"), Prog, line, sgr_dbname (), newsg.sg_name); - errors++; + errors = true; continue; } } @@ -608,7 +608,7 @@ int main (int argc, char **argv) fprintf (stderr, _("%s: line %jd: failed to prepare the new %s entry '%s'\n"), Prog, line, gr_dbname (), newgr.gr_name); - errors++; + errors = true; continue; } } @@ -621,7 +621,7 @@ int main (int argc, char **argv) * changes to be written out all at once, and then unlocked * afterwards. */ - if (0 != errors) { + if (errors) { fprintf (stderr, _("%s: error detected, changes ignored\n"), Prog); fail_exit (1); diff --git a/src/chpasswd.c b/src/chpasswd.c index 0ac7d38b3..5d9c42c10 100644 --- a/src/chpasswd.c +++ b/src/chpasswd.c @@ -453,7 +453,7 @@ int main (int argc, char **argv) bool use_pam = true; #endif /* USE_PAM */ - int errors = 0; + bool errors = false; intmax_t line = 0; log_set_progname(Prog); @@ -517,7 +517,7 @@ int main (int argc, char **argv) fprintf (stderr, _("%s: line %jd: line too long\n"), Prog, line); - errors++; + errors = true; continue; } } @@ -537,7 +537,7 @@ int main (int argc, char **argv) fprintf (stderr, _("%s: line %jd: missing new password\n"), Prog, line); - errors++; + errors = true; continue; } newpwd = cp; @@ -548,7 +548,7 @@ int main (int argc, char **argv) fprintf (stderr, _("%s: (line %jd, user %s) password not changed\n"), Prog, line, name); - errors++; + errors = true; } } else #endif /* USE_PAM */ @@ -577,7 +577,7 @@ int main (int argc, char **argv) fprintf (stderr, _("%s: line %jd: user '%s' does not exist\n"), Prog, line, name); - errors++; + errors = true; continue; } if (is_shadow_pwd) { @@ -643,7 +643,7 @@ int main (int argc, char **argv) fprintf (stderr, _("%s: line %jd: failed to prepare the new %s entry '%s'\n"), Prog, line, spw_dbname (), newsp.sp_namp); - errors++; + errors = true; continue; } } @@ -653,7 +653,7 @@ int main (int argc, char **argv) fprintf (stderr, _("%s: line %jd: failed to prepare the new %s entry '%s'\n"), Prog, line, pw_dbname (), newpw.pw_name); - errors++; + errors = true; continue; } } @@ -670,7 +670,7 @@ int main (int argc, char **argv) * With PAM, it is not possible to delay the update of the * password database. */ - if (0 != errors) { + if (errors) { #ifdef USE_PAM if (!use_pam) #endif /* USE_PAM */ diff --git a/src/grpck.c b/src/grpck.c index 6bac2849f..e959f0adb 100644 --- a/src/grpck.c +++ b/src/grpck.c @@ -74,15 +74,15 @@ static int check_members (const char *groupname, const char *fmt_info, const char *fmt_prompt, const char *fmt_syslog, - int *errors); -static void check_grp_file (int *errors, bool *changed); + bool *errors); +static void check_grp_file (bool *errors, bool *changed); #ifdef SHADOWGRP static void compare_members_lists (const char *groupname, char **members, char **other_members, const char *file, const char *other_file); -static void check_sgr_file (int *errors, bool *changed); +static void check_sgr_file (bool *errors, bool *changed); #endif /* @@ -360,7 +360,7 @@ static void close_files (bool changed) /* * check_members - check that every members of a group exist * - * If an error is detected, *errors is incremented. + * If an error is detected, *errors is set to true. * * The user will be prompted for the removal of the non-existent * user. @@ -381,7 +381,7 @@ static int check_members (const char *groupname, const char *fmt_info, const char *fmt_prompt, const char *fmt_syslog, - int *errors) + bool *errors) { int i; int members_changed = 0; @@ -398,7 +398,7 @@ static int check_members (const char *groupname, * Can't find this user. Remove them * from the list. */ - *errors += 1; + *errors = true; printf (fmt_info, groupname, members[i]); printf (fmt_prompt, members[i]); @@ -454,7 +454,7 @@ static void compare_members_lists (const char *groupname, /* * check_grp_file - check the content of the group file */ -static void check_grp_file (int *errors, bool *changed) +static void check_grp_file (bool *errors, bool *changed) { struct commonio_entry *gre, *tgre; struct group *grp; @@ -487,7 +487,7 @@ static void check_grp_file (int *errors, bool *changed) */ (void) puts (_("invalid group file entry")); printf (_("delete line '%s'? "), gre->line); - *errors += 1; + *errors = true; /* * prompt the user to delete the entry or not @@ -547,7 +547,7 @@ static void check_grp_file (int *errors, bool *changed) */ (void) puts (_("duplicate group entry")); printf (_("delete line '%s'? "), gre->line); - *errors += 1; + *errors = true; /* * prompt the user to delete the entry or not @@ -561,7 +561,7 @@ static void check_grp_file (int *errors, bool *changed) * Check for invalid group names. --marekm */ if (!is_valid_group_name (grp->gr_name)) { - *errors += 1; + *errors = true; printf (_("invalid group name '%s'\n"), grp->gr_name); } @@ -570,7 +570,7 @@ static void check_grp_file (int *errors, bool *changed) */ if (grp->gr_gid == (gid_t)-1) { printf (_("invalid group ID '%lu'\n"), (long unsigned int)grp->gr_gid); - *errors += 1; + *errors = true; } /* @@ -607,7 +607,7 @@ static void check_grp_file (int *errors, bool *changed) sgr_file); printf (_("add group '%s' in %s? "), grp->gr_name, sgr_file); - *errors += 1; + *errors = true; if (yes_or_no (read_only)) { struct sgrp sg; struct group gr; @@ -653,7 +653,7 @@ static void check_grp_file (int *errors, bool *changed) if (!streq(grp->gr_passwd, SHADOW_PASSWD_STRING)) { printf (_("group %s has an entry in %s, but its password field in %s is not set to 'x'\n"), grp->gr_name, sgr_file, grp_file); - *errors += 1; + *errors = true; } } } @@ -666,7 +666,7 @@ static void check_grp_file (int *errors, bool *changed) /* * check_sgr_file - check the content of the shadowed group file (gshadow) */ -static void check_sgr_file (int *errors, bool *changed) +static void check_sgr_file (bool *errors, bool *changed) { const struct group *grp; struct commonio_entry *sge, *tsge; @@ -690,7 +690,7 @@ static void check_sgr_file (int *errors, bool *changed) */ (void) puts (_("invalid shadow group file entry")); printf (_("delete line '%s'? "), sge->line); - *errors += 1; + *errors = true; /* * prompt the user to delete the entry or not @@ -750,7 +750,7 @@ static void check_sgr_file (int *errors, bool *changed) */ (void) puts (_("duplicate shadow group entry")); printf (_("delete line '%s'? "), sge->line); - *errors += 1; + *errors = true; /* * prompt the user to delete the entry or not @@ -768,7 +768,7 @@ static void check_sgr_file (int *errors, bool *changed) printf (_("no matching group file entry in %s\n"), grp_file); printf (_("delete line '%s'? "), sge->line); - *errors += 1; + *errors = true; if (yes_or_no (read_only)) { goto delete_sg; } @@ -816,7 +816,7 @@ static void check_sgr_file (int *errors, bool *changed) */ int main (int argc, char **argv) { - int errors = 0; + bool errors = false; bool changed = false; log_set_progname(Prog); @@ -863,7 +863,7 @@ int main (int argc, char **argv) /* * Tell the user what we did and exit. */ - if (0 != errors) { + if (errors) { if (changed) { printf (_("%s: the files have been updated\n"), Prog); } else { @@ -871,6 +871,6 @@ int main (int argc, char **argv) } } - return ((0 != errors) ? E_BAD_ENTRY : E_OKAY); + return (errors ? E_BAD_ENTRY : E_OKAY); } diff --git a/src/pwck.c b/src/pwck.c index ae7ddaddf..b485a5a87 100644 --- a/src/pwck.c +++ b/src/pwck.c @@ -73,8 +73,8 @@ NORETURN static void usage (int status); static void process_flags (int argc, char **argv); static void open_files (void); static void close_files (bool changed); -static void check_pw_file (int *errors, bool *changed); -static void check_spw_file (int *errors, bool *changed); +static void check_pw_file (bool *errors, bool *changed); +static void check_spw_file (bool *errors, bool *changed); extern int allow_bad_names; @@ -367,7 +367,7 @@ static void close_files (bool changed) /* * check_pw_file - check the content of the passwd file */ -static void check_pw_file (int *errors, bool *changed) +static void check_pw_file (bool *errors, bool *changed) { struct commonio_entry *pfe, *tpfe; struct passwd *pwd; @@ -399,7 +399,7 @@ static void check_pw_file (int *errors, bool *changed) */ puts (_("invalid password file entry")); printf (_("delete line '%s'? "), pfe->line); - *errors += 1; + *errors = true; /* * prompt the user to delete the entry or not @@ -460,7 +460,7 @@ static void check_pw_file (int *errors, bool *changed) */ puts (_("duplicate password entry")); printf (_("delete line '%s'? "), pfe->line); - *errors += 1; + *errors = true; /* * prompt the user to delete the entry or not @@ -482,7 +482,7 @@ static void check_pw_file (int *errors, bool *changed) printf(_("invalid user name '%s'\n"), pwd->pw_name); } - *errors += 1; + *errors = true; } /* @@ -490,7 +490,7 @@ static void check_pw_file (int *errors, bool *changed) */ if (pwd->pw_uid == (uid_t)-1) { printf (_("invalid user ID '%lu'\n"), (long unsigned int)pwd->pw_uid); - *errors += 1; + *errors = true; } /* @@ -505,7 +505,7 @@ static void check_pw_file (int *errors, bool *changed) printf (_("user '%s': no group %lu\n"), pwd->pw_name, (unsigned long) pwd->pw_gid); - *errors += 1; + *errors = true; } /* @@ -524,7 +524,7 @@ static void check_pw_file (int *errors, bool *changed) if (NULL == nonexistent || !streq(pwd->pw_dir, nonexistent)) { printf (_("user '%s': directory '%s' does not exist\n"), pwd->pw_name, pwd->pw_dir); - *errors += 1; + *errors = true; } } } @@ -541,7 +541,7 @@ static void check_pw_file (int *errors, bool *changed) */ printf (_("user '%s': program '%s' does not exist\n"), pwd->pw_name, pwd->pw_shell); - *errors += 1; + *errors = true; } /* @@ -556,10 +556,10 @@ static void check_pw_file (int *errors, bool *changed) pwd->pw_name); printf (_("create tcb directory for %s?"), pwd->pw_name); - *errors += 1; + *errors = true; if (yes_or_no (read_only)) { if (shadowtcb_create (pwd->pw_name, pwd->pw_uid) == SHADOWTCB_FAILURE) { - *errors += 1; + *errors = true; printf (_("failed to create tcb directory for %s\n"), pwd->pw_name); continue; } @@ -568,7 +568,7 @@ static void check_pw_file (int *errors, bool *changed) } } if (spw_lock () == 0) { - *errors += 1; + *errors = true; fprintf (stderr, _("%s: cannot lock %s.\n"), Prog, spw_dbname ()); @@ -579,7 +579,7 @@ static void check_pw_file (int *errors, bool *changed) fprintf (stderr, _("%s: cannot open %s\n"), Prog, spw_dbname ()); - *errors += 1; + *errors = true; if (spw_unlock () == 0) { fprintf (stderr, _("%s: failed to unlock %s\n"), @@ -601,7 +601,7 @@ static void check_pw_file (int *errors, bool *changed) spw_dbname ()); printf (_("add user '%s' in %s? "), pwd->pw_name, spw_dbname ()); - *errors += 1; + *errors = true; if (yes_or_no (read_only)) { struct spwd sp; struct passwd pw; @@ -650,7 +650,7 @@ static void check_pw_file (int *errors, bool *changed) && !streq(pwd->pw_passwd, SHADOW_PASSWD_STRING)) { printf (_("user %s has an entry in %s, but its password field in %s is not set to 'x'\n"), pwd->pw_name, spw_dbname (), pw_dbname ()); - *errors += 1; + *errors = true; } } } @@ -687,7 +687,7 @@ static void check_pw_file (int *errors, bool *changed) /* * check_spw_file - check the content of the shadowed password file (shadow) */ -static void check_spw_file (int *errors, bool *changed) +static void check_spw_file (bool *errors, bool *changed) { struct commonio_entry *spe, *tspe; struct spwd *spw; @@ -724,7 +724,7 @@ static void check_spw_file (int *errors, bool *changed) */ puts (_("invalid shadow password file entry")); printf (_("delete line '%s'? "), spe->line); - *errors += 1; + *errors = true; /* * prompt the user to delete the entry or not @@ -785,7 +785,7 @@ static void check_spw_file (int *errors, bool *changed) */ puts (_("duplicate shadow password entry")); printf (_("delete line '%s'? "), spe->line); - *errors += 1; + *errors = true; /* * prompt the user to delete the entry or not @@ -807,7 +807,7 @@ static void check_spw_file (int *errors, bool *changed) printf (_("no matching password file entry in %s\n"), pw_dbname ()); printf (_("delete line '%s'? "), spe->line); - *errors += 1; + *errors = true; /* * prompt the user to delete the entry or not @@ -826,7 +826,7 @@ static void check_spw_file (int *errors, bool *changed) && (spw->sp_lstchg > t / DAY)) { printf (_("user %s: last password change in the future\n"), spw->sp_namp); - *errors += 1; + *errors = true; } } } @@ -837,7 +837,7 @@ static void check_spw_file (int *errors, bool *changed) */ int main (int argc, char **argv) { - int errors = 0; + bool errors = false; bool changed = false; log_set_progname(Prog); @@ -890,13 +890,13 @@ int main (int argc, char **argv) /* * Tell the user what we did and exit. */ - if (0 != errors) { + if (errors) { printf (changed ? _("%s: the files have been updated\n") : _("%s: no changes\n"), Prog); } closelog (); - return ((0 != errors) ? E_BADENTRY : E_OKAY); + return (errors ? E_BADENTRY : E_OKAY); } diff --git a/src/useradd.c b/src/useradd.c index 7623dabd4..498619e31 100644 --- a/src/useradd.c +++ b/src/useradd.c @@ -758,7 +758,7 @@ err_free_new: static int get_groups (char *list) { struct group *grp; - int errors = 0; + bool errors = false; int ngroups = 0; /* @@ -808,7 +808,7 @@ static int get_groups (char *list) fprintf (stderr, _("%s: group '%s' does not exist\n"), Prog, g); - errors++; + errors = true; } /* @@ -842,7 +842,7 @@ static int get_groups (char *list) /* * Any errors in finding group names are fatal */ - if (0 != errors) { + if (errors) { return -1; } diff --git a/src/userdel.c b/src/userdel.c index 1e7367637..1703f0e05 100644 --- a/src/userdel.c +++ b/src/userdel.c @@ -119,7 +119,7 @@ static void user_cancel (const char *); static bool path_prefix (const char *, const char *); #endif /* EXTRA_CHECK_HOME_DIR */ static int is_owner (uid_t, const char *); -static int remove_mailbox (void); +static bool remove_mailbox (void); #ifdef WITH_TCB static int remove_tcbdir (const char *user_name, uid_t user_id); #endif /* WITH_TCB */ @@ -789,9 +789,10 @@ static int is_owner (uid_t uid, const char *path) return (st.st_uid == uid) ? 1 : 0; } -static int remove_mailbox (void) +static bool remove_mailbox (void) { - int i, errors = 0; + int i; + bool errors = false; char *mailfile; const char *maildir; @@ -844,7 +845,7 @@ static int remove_mailbox (void) "deleting mail file", user_name, user_id, SHADOW_AUDIT_FAILURE); #endif /* WITH_AUDIT */ - errors = 1; + errors = true; /* continue */ } #ifdef WITH_AUDIT @@ -887,7 +888,7 @@ static int remove_mailbox (void) "deleting mail file", user_name, user_id, SHADOW_AUDIT_FAILURE); #endif /* WITH_AUDIT */ - errors = 1; + errors = true; /* continue */ } #ifdef WITH_AUDIT @@ -951,7 +952,7 @@ static int remove_tcbdir (const char *user_name, uid_t user_id) */ int main (int argc, char **argv) { - int errors = 0; /* Error in the removal of the home directory */ + bool errors = false; /* Error in the removal of the home directory */ #ifdef ACCT_TOOLS_SETUID #ifdef USE_PAM @@ -1152,7 +1153,9 @@ int main (int argc, char **argv) update_groups (); if (rflg) { - errors += remove_mailbox (); + if (remove_mailbox ()) { + errors = true; + } } if (rflg) { int home_owned = is_owner (user_id, user_home); @@ -1166,7 +1169,7 @@ int main (int argc, char **argv) _("%s: %s not owned by %s, not removing\n"), Prog, user_home, user_name); rflg = 0; - errors++; + errors = true; /* continue */ } } @@ -1192,7 +1195,7 @@ int main (int argc, char **argv) _("%s: not removing directory %s (would remove home of user %s)\n"), Prog, user_home, pwd->pw_name); rflg = false; - errors++; + errors = true; /* continue */ break; } @@ -1205,7 +1208,7 @@ int main (int argc, char **argv) #ifdef WITH_BTRFS int is_subvolume = btrfs_is_subvolume (user_home); if (is_subvolume < 0) { - errors++; + errors = true; /* continue */ } else if (is_subvolume > 0) { @@ -1213,7 +1216,7 @@ int main (int argc, char **argv) fprintf (stderr, _("%s: error removing subvolume %s\n"), Prog, user_home); - errors++; + errors = true; /* continue */ } } @@ -1223,7 +1226,7 @@ int main (int argc, char **argv) fprintf (stderr, _("%s: error removing directory %s\n"), Prog, user_home); - errors++; + errors = true; /* continue */ } #ifdef WITH_AUDIT @@ -1236,7 +1239,7 @@ int main (int argc, char **argv) #endif /* WITH_AUDIT */ } #ifdef WITH_AUDIT - if (0 != errors) { + if (errors) { audit_logger (AUDIT_DEL_USER, Prog, "deleting home directory", user_name, AUDIT_NO_ID, @@ -1273,13 +1276,15 @@ int main (int argc, char **argv) } #ifdef WITH_TCB - errors += remove_tcbdir (user_name, user_id); + if (remove_tcbdir (user_name, user_id)) { + errors = true; + } #endif /* WITH_TCB */ nscd_flush_cache ("passwd"); nscd_flush_cache ("group"); sssd_flush_cache (SSSD_DB_PASSWD | SSSD_DB_GROUP); - return ((0 != errors) ? E_HOMEDIR : E_SUCCESS); + return (errors ? E_HOMEDIR : E_SUCCESS); } diff --git a/src/usermod.c b/src/usermod.c index 24c5a4d23..80ab4989e 100644 --- a/src/usermod.c +++ b/src/usermod.c @@ -219,7 +219,7 @@ extern int allow_bad_names; static int get_groups (char *list) { struct group *grp; - int errors = 0; + bool errors = false; int ngroups = 0; /* @@ -257,7 +257,7 @@ static int get_groups (char *list) if (NULL == grp) { fprintf (stderr, _("%s: group '%s' does not exist\n"), Prog, g); - errors++; + errors = true; } /* @@ -288,7 +288,7 @@ static int get_groups (char *list) /* * Any errors in finding group names are fatal */ - if (0 != errors) { + if (errors) { return -1; } -- 2.47.3