Le mercredi 12 mai 2010 à 01:45 -0400, Hajime Taira a écrit :
> > I knew, my point is why do you try to open it as a file first,
> > then as a directory? If you really have to do so, how about
> > using stat() to check if it's a file first then do open()?
>
> Ok, done. I changed open() to stat().
>
> Thanks
>
> --- util-linux-ng.old/fsfreeze/fsfreeze.c 1970-01-01 09:00:00.000000000 +0900
> +++ util-linux-ng/fsfreeze/fsfreeze.c 2010-05-12 14:30:21.000000000 +0900
> @@ -0,0 +1,94 @@
> +/* fsfreeze.c -- Filesystem freeze/unfreeze IO for Linux
> + *
> + * Copyright (C) 2010 Hajime Taira (htaira@xxxxxxxxxx)
> + * Masatake Yamato (yamato@xxxxxxxxxx)
> + *
> + * This program is free software. You can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation: either version 1 or
> + * (at your option) any later version.
> + */
> +
> +
> +#include <stdio.h>
> +#include <errno.h>
> +#include <string.h>
> +#include <fcntl.h>
> +#include <linux/fs.h>
> +#include <sys/ioctl.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +
> +int freeze_f(int fd)
> +{
> + return ioctl(fd, FIFREEZE, 0);
> +}
> +
> +int unfreeze_f(int fd)
> +{
> + return ioctl(fd, FITHAW, 0);
> +}
> +
> +void usage(void)
> +{
> + fprintf(stderr, "Usage: fsfreeze -f | -u <mount point>\n");
> + fprintf(stderr, "\n");
> +}
> +
> +int main(int argc, char **argv)
> +{
> + int fd;
> + int ret = -1;
A return value is only valid between 0 and 255. See macro WEXITSTATUS()
in wait(2) man page. main() should not return -1.
You could return 1 for invalid options, 2 for invalid file, 3 for
invalid ioctl and 0 for OK.
> + char *path;
> + struct stat sb;
> +
> + if(argc == 3) {
You could indent less using such construct:
if (argc != 3) {
usage();
return ret;
}
> + path = argv[2];
> +
> + if(stat(path, &sb) == -1) {
> + perror(path);
> + return ret;
> + }
> +
Why not opening the path first, then using fstat(), just to close any
race condition here.
> + if((sb.st_mode & S_IFMT) == S_IFDIR) {
> + fd = open(path, O_RDONLY);
> + if (fd < 0) {
> + perror(path);
> + return ret;
> + }
Same here:
if((sb.st_mode & S_IFMT) != S_IFDIR) {
usage();
return ret;
}
> + } else {
> + usage();
> + return ret;
> + }
> +
> + /* freeze operation */
> + if(strcmp(argv[1],"-f") == 0) {
> + ret = freeze_f(fd);
> + if (ret != 0) {
> + perror("freeze");
> + close(fd);
> + return ret;
> + }
> +
> + }
> + /* unfreeze operation */
> + else if(strcmp(argv[1],"-u") == 0) {
> + ret = unfreeze_f(fd);
> + if (ret != 0) {
> + perror("unfreeze");
> + close(fd);
> + return ret;
> + }
You should check parameter consistency first, e.g. don't open argv[2]
until you've checked argv[1].
> + } else {
> + close(fd);
> + usage();
> + return ret;
> + }
> +
> + close(fd);
> + } else {
> + usage();
> + }
> +
> + return ret;
> +}
>
Regards.
--
Yann Droneaud
--
To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
