On Tue, Apr 03, 2007 at 11:53:16AM +0200, Tim Dijkstra wrote:
> On Tue, 3 Apr 2007 11:25:23 +0200
> Stefan Seyfried <[EMAIL PROTECTED]> wrote:
>
> > Hi,
> >
> > This is a RFC, i am not sure if printing the program name in all error
> > messages is really useful, or if just "suspend:" is better there, but
> > at least in usage() it should be the correct one.
>
> I think I would vote for not printing the program name at all in the
> informational messages, only in the error messages. But I haven't given
I think it is useful, because you can clearly see where those messages come
from. Users will sometimes see kernel messages and s2disk messages mixed and
it might be hard for them to tell which is which.
And yes, this is where argv[0] comes in more handy then plain "suspend", since
it is more exact.
> it much thought and really don't care much;) If we do want to print
> something there, I do want to have the argv[0] there, not suspend, else
> it would be a bit confusing.
>
> A few small points:
>
> Maybe you can fix the message on line suspend.c:187 too?
ok, i'd change it to (not resending whole diff):
static inline loff_t check_free_swap(int dev)
{
int error;
loff_t free_swap;
error = ioctl(dev, SNAPSHOT_AVAIL_SWAP, &free_swap);
if (!error)
return free_swap;
else
suspend_error("check_free_swap failed.");
return 0;
}
> > Index: suspend.c
> > ===================================================================
> > RCS file: /cvsroot/suspend/suspend/suspend.c,v
> > retrieving revision 1.73
> > diff -u -p -r1.73 suspend.c
> > --- suspend.c 1 Apr 2007 22:03:29 -0000 1.73
> > +++ suspend.c 2 Apr 2007 16:14:53 -0000
> > @@ -51,7 +51,7 @@
> >
> > #define suspend_error(msg, args...) \
> > do { \
> > - fprintf(stderr, "suspend: " msg " Reason: %m\n", ## args); \
> > + fprintf(stderr, "%s: " msg " Reason: %m\n", my_name, ## args); \
> > } while (0);
>
> What is this do { } while (0); thing good for?
gcc magic, there is a good reason for it (something about breaking build
instead of misbehaving at runtime), but i always foreget it.
> > @@ -733,8 +734,8 @@ static void suspend_shutdown(int snapsho
> > } else if (use_platform_suspend) {
> > int ret = platform_enter(snapshot_fd);
> > if (ret < 0)
> > - fprintf(stderr, "suspend: pm_ops->enter returned"
> > - " error %d, calling power_off\n", errno);
> > + fprintf(stderr, "%s: pm_ops->enter returned error "
> > + "%d, calling power_off\n", my_name, errno);
> > }
>
> Can't you use the suspend_error macro here?
yes, but it would have changed the output from
s2disk: pm_ops->enter returned 22, calling power_off
to
s2disk: pm_ops->enter failed, calling power_off. Reason: No such device
but since nobody is supposed to see this message anyway, i'll change it to:
if (ret < 0)
suspend_error("pm_ops->enter failed, calling
power_off.");
> > @@ -773,13 +774,13 @@ int suspend_system(int snapshot_fd, int
> > if (use_platform_suspend) {
> > int ret = platform_prepare(snapshot_fd);
> > if (ret < 0) {
> > - fprintf(stderr, "suspend: pm_ops->prepare returned "
> > - "error %d\n", errno);
> > + fprintf(stderr, "%s: pm_ops->prepare returned error "
> > + "%d\n", my_name, errno);
> > use_platform_suspend = 0;
> > }
> > }
>
> And here?
The same. Basically, when introducing suspend_error, i wanted to not change
the error texts. Changed to:
if (ret < 0) {
suspend_error("pm_ops->prepare failed, using "
"'shutdown mode = shutdown'.");
use_platform_suspend = 0;
}
--
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
