Hi
Here are some small changes to quotaon(8). If you want I can split
them up, but since they are small I am sending one diff. Here is
a list of changes roughly in the order they appear in the diff:
- Constify some function arguments
- Use __progname instead of separate whoami variable + small KNF
where the line is touched anyways.
- Order letters in getopt(3) call
- Convert a fprintf(3) + perror(3) to warn(3). warn(3) is
already used in this function for similar purposes.
- Replace strtok(3) with strsep(3). Is there a more
elegant way than my solution? (I took the freedom to collect all
the char * variables into one line).
- Set cp to NULL at the end of the while loop scanning the mount
options. Otherwise if the last mount option contains a '=' the
part after it is used as the quota file (of course only if the
desired userquota/groupquota option isn't found).
- Use strncmp(3) instead of memcmp(3). Looking at the kernel code
it seems that it is zero terminated. The other code nearby uses
strcmp(3) already.
- Invert an if condition to prevent an empty body.
Happy to hear feedback, also feel free to only commit parts of it.
Best,
Martin
P.S.: If the prototypes are touched anyways, the names of the
arguments might be removed as well to comply with KNF. Let me
know if you want that.
Index: quotaon.c
===================================================================
RCS file: /cvs/src/usr.sbin/quotaon/quotaon.c,v
retrieving revision 1.27
diff -u -p -r1.27 quotaon.c
--- quotaon.c 26 Apr 2018 12:42:51 -0000 1.27
+++ quotaon.c 29 Oct 2021 07:46:25 -0000
@@ -44,6 +44,8 @@
#include <fstab.h>
#include <unistd.h>
+extern char *__progname;
+
char *qfname = QUOTAFILENAME;
char *qfextension[] = INITQFNAMES;
@@ -52,34 +54,31 @@ int gflag; /* operate on group quotas *
int uflag; /* operate on user quotas */
int vflag; /* verbose */
-void usage(char *whoami);
-int hasquota(struct fstab *fs, int type, char **qfnamep, int force);
-int quotaonoff(struct fstab *fs, int offmode, int type, char *qfpathname);
-int oneof(char *target, char *list[], int cnt);
-int readonly(struct fstab *fs);
+void usage();
+int hasquota(const struct fstab *fs, int type, char **qfnamep, int force);
+int quotaonoff(const struct fstab *fs, int offmode, int type, char
*qfpathname);
+int oneof(const char *target, char *list[], int cnt);
+int readonly(const struct fstab *fs);
int
main(int argc, char *argv[])
{
struct fstab *fs;
- char *qfnp, *whoami;
+ char *qfnp;
long argnum, done = 0;
int i, offmode = 0, errs = 0;
extern int optind;
int ch;
- whoami = strrchr(*argv, '/') + 1;
- if (whoami == (char *)1)
- whoami = *argv;
- if (strcmp(whoami, "quotaoff") == 0)
+ if (strcmp(__progname, "quotaoff") == 0)
offmode = 1;
- else if (strcmp(whoami, "quotaon") != 0) {
+ else if (strcmp(__progname, "quotaon") != 0) {
fprintf(stderr, "Name must be quotaon or quotaoff not %s\n",
- whoami);
+ __progname);
exit(1);
}
- while ((ch = getopt(argc, argv, "avug")) != -1) {
+ while ((ch = getopt(argc, argv, "aguv")) != -1) {
switch (ch) {
case 'a':
aflag = 1;
@@ -94,13 +93,13 @@ main(int argc, char *argv[])
vflag = 1;
break;
default:
- usage(whoami);
+ usage();
}
}
argc -= optind;
argv += optind;
if (argc <= 0 && !aflag)
- usage(whoami);
+ usage();
if (!gflag && !uflag) {
gflag = 1;
uflag = 1;
@@ -142,22 +141,20 @@ main(int argc, char *argv[])
}
void
-usage(char *whoami)
+usage()
{
-
- fprintf(stderr, "usage: %s [-aguv] filesystem ...\n", whoami);
+ fprintf(stderr, "usage: %s [-aguv] filesystem ...\n", __progname);
exit(1);
}
int
-quotaonoff(struct fstab *fs, int offmode, int type, char *qfpathname)
+quotaonoff(const struct fstab *fs, int offmode, int type, char *qfpathname)
{
if (strcmp(fs->fs_file, "/") && readonly(fs))
return (1);
if (offmode) {
if (quotactl(fs->fs_file, QCMD(Q_QUOTAOFF, type), 0, 0) < 0) {
- fprintf(stderr, "quotaoff: ");
- perror(fs->fs_file);
+ warn("%s", fs->fs_file);
return (1);
}
if (vflag)
@@ -180,7 +177,7 @@ quotaonoff(struct fstab *fs, int offmode
* Check to see if target appears in list of size cnt.
*/
int
-oneof(char *target, char *list[], int cnt)
+oneof(const char *target, char *list[], int cnt)
{
int i;
@@ -194,10 +191,9 @@ oneof(char *target, char *list[], int cn
* Check to see if a particular quota is to be enabled.
*/
int
-hasquota(struct fstab *fs, int type, char **qfnamep, int force)
+hasquota(const struct fstab *fs, int type, char **qfnamep, int force)
{
- char *opt;
- char *cp;
+ char *opt, *cp, *pbuf;
static char initname, usrname[100], grpname[100];
static char buf[BUFSIZ];
@@ -209,13 +205,15 @@ hasquota(struct fstab *fs, int type, cha
initname = 1;
}
strlcpy(buf, fs->fs_mntops, sizeof buf);
- for (opt = strtok(buf, ","); opt; opt = strtok(NULL, ",")) {
+ pbuf = buf;
+ while ((opt = strsep(&pbuf, ","))) {
if ((cp = strchr(opt, '=')) != NULL)
*cp++ = '\0';
if (type == USRQUOTA && strcmp(opt, usrname) == 0)
break;
if (type == GRPQUOTA && strcmp(opt, grpname) == 0)
break;
+ cp = NULL;
}
if (!force && !opt)
return (0);
@@ -234,17 +232,15 @@ hasquota(struct fstab *fs, int type, cha
* MFS is special -- it puts "mfs:" in the kernel's mount table
*/
int
-readonly(struct fstab *fs)
+readonly(const struct fstab *fs)
{
struct statfs fsbuf;
if (statfs(fs->fs_file, &fsbuf) < 0 ||
strcmp(fsbuf.f_mntonname, fs->fs_file) ||
strcmp(fsbuf.f_mntfromname, fs->fs_spec)) {
- if (strcmp(fs->fs_file, "mfs") ||
- memcmp(fsbuf.f_mntfromname, "mfs:", sizeof("mfs:")-1))
- ;
- else {
+ if (strcmp(fs->fs_file, "mfs") == 0 &&
+ strncmp(fsbuf.f_mntfromname, "mfs:", 4) == 0) {
printf("%s: not mounted\n", fs->fs_file);
return (1);
}
