Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify usermod.c #454

Closed
alejandro-colomar opened this issue Dec 14, 2021 · 2 comments
Closed

Simplify usermod.c #454

alejandro-colomar opened this issue Dec 14, 2021 · 2 comments

Comments

@alejandro-colomar
Copy link
Collaborator

alejandro-colomar commented Dec 14, 2021

In parsing the argument to the --expiredate option, there's a special case for "":

if ('\0' != *optarg) {

			case 'e':
				if ('\0' != *optarg) {
					user_newexpire = strtoday (optarg);
					if (user_newexpire < -1) {
						fprintf (stderr,
						         _("%s: invalid date '%s'\n"),
						         Prog, optarg);
						exit (E_BAD_ARG);
					}
					user_newexpire *= DAY / SCALE;
				} else {
					user_newexpire = -1;
				}
				eflg = true;
				break;

However, (the new version of) strtoday() already handles that special case:

if ((NULL == str) || ('\0' == *str)) {

	if ((NULL == str) || ('\0' == *str)) {
		return -1;
	}

	...

So the calling code could be simplified to the following:

			case 'e':
				user_newexpire = strtoday (optarg);
				if (user_newexpire < -1) {
					fprintf (stderr,
					         _("%s: invalid date '%s'\n"),
					         Prog, optarg);
					exit (E_BAD_ARG);
				}
				user_newexpire *= DAY / SCALE;
				eflg = true;
				break;

There are 2 versions of strtoday() (actually 3), and at first I thought that the old ones might not handle correctly the special case. However, I found what could be a bug, which I think would handle "" correctly by accident. Instead of returning -2 on error, the old functions return -1, and since they don't handle "" specially, they will report an error. Since -1 is handled not as an error but as a 'never' by the calling code, it would work.

I'll send a pull request soon and link to this issue.

@ikerexxe
Copy link
Collaborator

ikerexxe commented Jan 4, 2022

As the PR was merged I guess you could close this issue. Am I right?

@alejandro-colomar
Copy link
Collaborator Author

Ahh, I forgot. Sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants