On Wed, 2010-12-01 at 17:18 +0100, Marek Polacek wrote:
> Signed-off-by: Marek Polacek <mmpolacek@xxxxxxxxx>
> ---
> login-utils/chfn.c | 59
> ++++++++++++++++++++++++---------------------------
> 1 files changed, 28 insertions(+), 31 deletions(-)
>
> diff --git a/login-utils/chfn.c b/login-utils/chfn.c
> index ef0a746..634628f 100644
> --- a/login-utils/chfn.c
> +++ b/login-utils/chfn.c
> @@ -31,6 +31,7 @@
> #include <unistd.h>
> #include <pwd.h>
> #include <errno.h>
> +#include <err.h>
> #include <ctype.h>
> #include <getopt.h>
> #include "my_crypt.h"
> @@ -55,7 +56,7 @@
> if ((_rc) != PAM_SUCCESS) { \
> fprintf(stderr, "\n%s\n", pam_strerror((_ph), (_rc))); \
> pam_end((_ph), (_rc)); \
> - exit(1); \
> + exit(EXIT_FAILURE); \
> } \
> } while(0)
>
> @@ -131,20 +132,20 @@ int main (int argc, char **argv) {
> parse_passwd (getpwuid (uid), &oldf);
> if (! oldf.username) {
> fprintf (stderr, _("%s: you (user %d) don't exist.\n"), whoami, uid);
> - return (-1); }
> + return EXIT_FAILURE;
> + }
> }
> else {
> parse_passwd (getpwnam (newf.username), &oldf);
> if (! oldf.username) {
> cp = newf.username;
> fprintf (stderr, _("%s: user \"%s\" does not exist.\n"), whoami, cp);
> - return (-1); }
> + return EXIT_FAILURE;
> + }
> }
>
> - if (!(is_local(oldf.username))) {
> - fprintf (stderr, _("%s: can only change local entries.\n"), whoami);
> - exit(1);
> - }
> + if (!(is_local(oldf.username)))
> + errx(EXIT_FAILURE, _("can only change local entries"));
>
> #ifdef HAVE_LIBSELINUX
> if (is_selinux_enabled() > 0) {
> @@ -156,22 +157,18 @@ int main (int argc, char **argv) {
> fprintf(stderr, _("%s: %s is not authorized to change the finger
> info of %s\n"),
> whoami, user_context, oldf.username);
> freecon(user_context);
> - exit(1);
> + exit(EXIT_FAILURE);
> }
> }
> - if (setupDefaultContext("/etc/passwd") != 0) {
> - fprintf(stderr,_("%s: Can't set default context for /etc/passwd"),
> - whoami);
> - exit(1);
> - }
> + if (setupDefaultContext("/etc/passwd"))
> + errx(EXIT_FAILURE, _("can't set default context for /etc/passwd"));
> }
> #endif
>
> /* Reality check */
> if (uid != 0 && uid != oldf.pw->pw_uid) {
> errno = EACCES;
> - perror (whoami);
> - return (-1);
> + err(EXIT_FAILURE, NULL);
> }
>
> printf (_("Changing finger information for %s.\n"), oldf.username);
> @@ -187,7 +184,7 @@ int main (int argc, char **argv) {
> if(retcode != PAM_SUCCESS) {
> fprintf(stderr, _("%s: PAM failure, aborting: %s\n"),
> whoami, pam_strerror(pamh, retcode));
> - exit(1);
> + exit(EXIT_FAILURE);
> }
>
> retcode = pam_authenticate(pamh, 0);
> @@ -212,7 +209,7 @@ int main (int argc, char **argv) {
> if(strncmp(oldf.pw->pw_passwd,
> crypt(pwdstr, oldf.pw->pw_passwd), 13)) {
> puts(_("Incorrect password."));
> - exit(1);
> + exit(EXIT_FAILURE);
> }
> }
> # endif /* HAVE_SECURITY_PAM_MISC_H */
> @@ -223,7 +220,7 @@ int main (int argc, char **argv) {
>
> if (! set_changed_data (&oldf, &newf)) {
> printf (_("Finger information not changed.\n"));
> - return 0;
> + return EXIT_SUCCESS;
> }
> status = save_new_data (&oldf);
> return status;
> @@ -260,16 +257,16 @@ static boolean parse_argv (argc, argv, pinfo)
> /* version? output version and exit. */
> if (c == 'v') {
> printf ("%s\n", PACKAGE_STRING);
> - exit (0);
> + exit (EXIT_SUCCESS);
> }
> if (c == 'u') {
> usage (stdout);
> - exit (0);
> + exit (EXIT_SUCCESS);
> }
> /* all other options must have an argument. */
> if (! optarg) {
> usage (stderr);
> - exit (-1);
> + exit (EXIT_FAILURE);
> }
> /* ok, we were given an argument */
> info_given = true;
> @@ -310,7 +307,7 @@ static boolean parse_argv (argc, argv, pinfo)
> if (optind < argc) {
> if (optind + 1 < argc) {
> usage (stderr);
> - exit (-1);
> + exit (EXIT_FAILURE);
> }
> pinfo->username = argv[optind];
> }
> @@ -398,7 +395,7 @@ static char *prompt (question, def_val)
> *buf = 0;
> if (fgets (buf, sizeof (buf), stdin) == NULL) {
> printf (_("\nAborted.\n"));
> - exit (-1);
> + exit (EXIT_FAILURE);
> }
> /* remove the newline at the end of buf. */
> ans = buf;
> @@ -439,15 +436,15 @@ static int check_gecos_string (msg, gecos)
> if (c == ',' || c == ':' || c == '=' || c == '"' || c == '\n') {
> if (msg) printf ("%s: ", msg);
> printf (_("'%c' is not allowed.\n"), c);
> - return (-1);
> + return -1;
> }
> if (iscntrl (c)) {
> if (msg) printf ("%s: ", msg);
> printf (_("Control characters are not allowed.\n"));
> - return (-1);
> + return -1;
> }
> }
> - return (0);
> + return 0;
> }
>
> /*
> @@ -509,7 +506,7 @@ static int save_new_data (pinfo)
> if (setpwnam (pinfo->pw) < 0) {
> perror ("setpwnam");
> printf( _("Finger information *NOT* changed. Try again later.\n" ));
> - return (-1);
> + return -1;
> }
> printf (_("Finger information changed.\n"));
> return 0;
> @@ -517,6 +514,7 @@ static int save_new_data (pinfo)
>
> /*
> * xmalloc () -- malloc that never fails.
> + * TODO: Use xmalloc.h instead
Since you've already been cleaning up the return codes, why not use it
right away and get rid of this local xmalloc()?
Thanks,
Davidlohr
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
