Microcom: new applet, mount: make better use of fstab
Denys Vlasenko
vda.linux at googlemail.com
Wed Sep 12 12:06:56 UTC 2007
On Wednesday 12 September 2007 08:18, Vladimir Dronnikov wrote:
> Hello, Denis!
>
> 1) microcom
+ // compose lock file name
+ if ( !device ) return ENODEV;
+ if ( strchr(device, '/') ) device = strrchr(device, '/')+1;
+ if ( !*device ) return ENODEV;
+ device_lock_file = xasprintf("/var/lock/LCK..%s", device);
+
+ // someone has already locked the device?
+ // ...yes? nothing doing
+ if ( !stat(device_lock_file, &st) ) {
+ return EEXIST;
+ }
+
+ // seems device is free -> lock it now!
+ fd = open(device_lock_file, O_CREAT | O_RDWR, 0640);
+ if ( fd < 0 ) {
+ return EPERM;
+ }
+ s = xasprintf("%10d\n", getpid());
This is racy, and lacks O_TRUNC too.
Don't stat. Simply use open(..., O_WRONLY|O_CREAT|O_TRUNC|O_EXCL, 0644)
If it fails, then file probably exists (because of O_EXCL).
If it succeeds, there was no file and you already created a new one.
Why "%10d" and not "%d"?
I also wonder what will happen if user *can* open /dev/ttyS0,
but can't create /var/lock/LCK..ttyS0 because of permissions.
Something like -f(orce) flag can be useful.
+ unlink(device_lock_file);
+ if (ENABLE_FEATURE_CLEAN_UP && device_lock_file) free(device_lock_file);
Superfluous check for device_lock_file != NULL. if device_lock_file == NULL,
unlink will segfault before you reach if().
+ if ( (errno=lock_device(argv[0])) ) {
+ bb_perror_msg("Can't lock device");
Why do you invent error code (EPERM)? The reason can be different
(say, /var/lock doesn't exist).
I think better way would be to explicitly say
"cannot create lock file FILE: <errno>" - less confusing.
+ len = read(STDIN_FILENO, bb_common_bufsiz1, 100); if (len > 0) write(fd, bb_common_bufsiz1, len);
Not very readable.
+ do {
+ fd_set rfs;
+
+ FD_ZERO(&rfs);
+ FD_SET(STDIN_FILENO, &rfs);
+ FD_SET(fd, &rfs);
+
+ select(FD_SETSIZE, &rfs, NULL, NULL, NULL);
When you have fixed set of fds to check, code using poll
tends to ba smaller by ~100 bytes than select.
These FD_xxx macros produce nasty big code, and fd_set uses 128 bytes on stack.
+ puts("goodbye!\r\n");
Can this fool user into thinking that it came from remote machine?
--
vda
More information about the busybox
mailing list