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 _______________________________________________ busybox mailing list busybox@busybox.net http://busybox.net/cgi-bin/mailman/listinfo/busybox