Hi, after reading this thread, i got the impression that we do not report errors verbosely enough. I mean - we normally never see those messages, but if we do, due to an error having happened, we always need to start adding more debugging printf's before we know what happened.
Oh, yes, and pm-ops->* can only return -1 or 0, so telling the user that it returned "-1" is also pretty pointless, let's tell him whats in errno instead. So i came up with this one, comments are appreciated: Index: suspend.c =================================================================== RCS file: /cvsroot/suspend/suspend/suspend.c,v retrieving revision 1.70 diff -u -p -r1.70 suspend.c --- suspend.c 16 Mar 2007 16:02:22 -0000 1.70 +++ suspend.c 26 Mar 2007 09:03:09 -0000 @@ -49,6 +49,11 @@ #include "s2ram.h" #endif +#define suspend_error(err, msg, args...) \ +do { \ + fprintf(stderr, "suspend: " msg " Reason: %s\n", ## args, strerror(err)); \ +} while (0); + static char snapshot_dev_name[MAX_STR_LEN] = SNAPSHOT_DEVICE; static char resume_dev_name[MAX_STR_LEN] = RESUME_DEVICE; static loff_t resume_offset; @@ -729,7 +734,7 @@ static void suspend_shutdown(int snapsho int ret = platform_enter(snapshot_fd); if (ret < 0) fprintf(stderr, "suspend: pm_ops->enter returned" - " error %d, calling power_off\n", ret); + " error %d, calling power_off\n", errno); } power_off(); /* Signature is on disk, it is very dangerous to continue now. @@ -769,7 +774,7 @@ int suspend_system(int snapshot_fd, int int ret = platform_prepare(snapshot_fd); if (ret < 0) { fprintf(stderr, "suspend: pm_ops->prepare returned " - "error %d\n", ret); + "error %d\n", errno); use_platform_suspend = 0; } } @@ -906,13 +911,13 @@ static int prepare_console(int *orig_vc, return fd; error = ioctl(fd, VT_ACTIVATE, vt); if (error) { - fprintf(stderr, "Could not activate the VT %d\n", vt); + suspend_error(errno, "Could not activate the VT %d.", vt); fflush(stderr); goto Close_fd; } error = ioctl(fd, VT_WAITACTIVE, vt); if (error) { - fprintf(stderr, "VT %d activation failed\n", vt); + suspend_error(errno, "VT %d activation failed.", vt); fflush(stderr); goto Close_fd; } @@ -932,8 +937,8 @@ static int prepare_console(int *orig_vc, tiocl[0] = TIOCL_SETKMSGREDIRECT; tiocl[1] = vt; if (ioctl(fd, TIOCLINUX, tiocl)) { - fprintf(stderr, "Failed to redirect kernel messages to VT %d\n" - "Reason: %s\n", vt, strerror(errno)); + suspend_error(errno, + "Failed to redirect kernel messages to VT %d.", vt); fflush(stderr); set_kmsg_redirect = 0; } @@ -956,13 +961,13 @@ static void restore_console(int fd, int error = ioctl(fd, VT_ACTIVATE, orig_vc); if (error) { - fprintf(stderr, "Could not activate the VT %d\n", orig_vc); + suspend_error(errno, "Could not activate the VT %d.", orig_vc); fflush(stderr); goto Close_fd; } error = ioctl(fd, VT_WAITACTIVE, orig_vc); if (error) { - fprintf(stderr, "VT %d activation failed\n", orig_vc); + suspend_error(errno, "VT %d activation failed.", orig_vc); fflush(stderr); } if (set_kmsg_redirect) { @@ -1233,7 +1238,7 @@ int main(int argc, char *argv[]) ret = open("/dev/null", O_RDWR); if (ret < 0) { ret = errno; - fprintf(stderr, "suspend: Could not open /dev/null\n"); + suspend_error(ret, "Could not open /dev/null."); return ret; } } while (ret < 3); @@ -1272,7 +1277,7 @@ int main(int argc, char *argv[]) mem_pool = malloc(mem_size); if (!mem_pool) { ret = errno; - fprintf(stderr, "suspend: Could not allocate memory\n"); + suspend_error(ret, "Could not allocate memory."); return ret; } #ifdef CONFIG_ENCRYPT @@ -1300,20 +1305,20 @@ int main(int argc, char *argv[]) if (mlockall(MCL_CURRENT | MCL_FUTURE)) { ret = errno; - fprintf(stderr, "suspend: Could not lock myself\n"); + suspend_error(ret, "Could not lock myself."); return ret; } snprintf(chroot_path, MAX_STR_LEN, "/proc/%d", getpid()); if (mount("none", chroot_path, "tmpfs", 0, NULL)) { ret = errno; - fprintf(stderr, "suspend: Could not mount tmpfs on %s\n", chroot_path); + suspend_error(ret, "Could not mount tmpfs on %s.", chroot_path); return ret; } ret = 0; if (stat(resume_dev_name, &stat_buf)) { - fprintf(stderr, "suspend: Could not stat the resume device file\n"); + suspend_error(errno, "Could not stat the resume device file."); ret = ENODEV; goto Umount; } @@ -1324,26 +1329,26 @@ int main(int argc, char *argv[]) } if (chdir(chroot_path)) { ret = errno; - fprintf(stderr, "suspend: Could not change directory to %s\n", + suspend_error(ret, "Could not change directory to %s.", chroot_path); goto Umount; } resume_dev = stat_buf.st_rdev; if (mknod("resume", S_IFBLK | 0600, resume_dev)) { ret = errno; - fprintf(stderr, "suspend: Could not create %s/%s\n", + suspend_error(ret, "Could not create %s/%s.", chroot_path, "resume"); goto Umount; } resume_fd = open("resume", O_RDWR); if (resume_fd < 0) { ret = errno; - fprintf(stderr, "suspend: Could not open the resume device\n"); + suspend_error(ret, "Could not open the resume device."); goto Umount; } if (stat(snapshot_dev_name, &stat_buf)) { - fprintf(stderr, "suspend: Could not stat the snapshot device file\n"); + suspend_error(errno, "Could not stat the snapshot device file."); ret = ENODEV; goto Close_resume_fd; } @@ -1356,21 +1361,21 @@ int main(int argc, char *argv[]) snapshot_fd = open(snapshot_dev_name, O_RDONLY); if (snapshot_fd < 0) { ret = errno; - fprintf(stderr, "suspend: Could not open the snapshot device\n"); + suspend_error(ret, "Could not open the snapshot device."); goto Close_resume_fd; } if (set_swap_file(snapshot_fd, resume_dev, resume_offset)) { ret = errno; - fprintf(stderr, "suspend: Could not use the resume device " - "(try swapon -a)\n"); + suspend_error(ret, "Could not use the resume device " + "(try swapon -a)."); goto Close_snapshot_fd; } vt_fd = prepare_console(&orig_vc, &suspend_vc); if (vt_fd < 0) { ret = errno; - fprintf(stderr, "suspend: Could not open a virtual terminal\n"); + suspend_error(ret, "Could not open a virtual terminal."); goto Close_snapshot_fd; } @@ -1378,7 +1383,7 @@ int main(int argc, char *argv[]) if (lock_vt() < 0) { ret = errno; - fprintf(stderr, "suspend: Could not lock the terminal\n"); + suspend_error(ret, "Could not lock the terminal."); goto Restore_console; } @@ -1435,7 +1440,7 @@ Close_resume_fd: Umount: if (chdir("/")) { ret = errno; - fprintf(stderr, "suspend: Could not change directory to /\n"); + suspend_error(ret, "Could not change directory to /"); } else { umount(chroot_path); } -- Stefan Seyfried "Any ideas, John?" "Well, surrounding them's out." ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ Suspend-devel mailing list Suspend-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/suspend-devel