Send plymouth mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
http://lists.freedesktop.org/mailman/listinfo/plymouth
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of plymouth digest..."
Today's Topics:
1. Re: [PATCH 3/9] add support for long data types
(William Jon McCann)
2. Re: [PATCH 1/9] improve the layout of the help output (Ray Strode)
3. Re: [PATCH 7/9] add a message display method (Ray Strode)
4. Re: [PATCH 8/9] add debug command line option (Ray Strode)
5. Re: [PATCH 9/9] add way to detect if an option was set or not
(Ray Strode)
6. Re: [PATCH 5/9] add a mode commandline option (Charlie Brej)
7. Re: [PATCH 5/9] add a mode commandline option (William Jon McCann)
----------------------------------------------------------------------
Message: 1
Date: Mon, 23 Feb 2009 16:35:48 -0500
From: William Jon McCann <[email protected]>
Subject: Re: [PATCH 3/9] add support for long data types
To: Ray Strode <[email protected]>
Cc: [email protected], William Jon McCann
<[email protected]>
Message-ID:
<[email protected]>
Content-Type: text/plain; charset=ISO-8859-1
Hey,
On Mon, Feb 23, 2009 at 4:15 PM, Ray Strode <[email protected]> wrote:
> Hi,
>
> On Mon, Feb 23, 2009 at 3:35 PM, <[email protected]> wrote:
>> From: William Jon McCann <[email protected]>
>
> Could use a description here. Maybe just say why you need longs supported?
>
> If this is just for the ptmx variable, i'd say lets drop this patch entirely
> and
> make ptmx an int like it should have been to begin with.
Yeah it was only for ptmx. If we change that to an int this isn't needed.
Jon
------------------------------
Message: 2
Date: Mon, 23 Feb 2009 16:37:00 -0500
From: Ray Strode <[email protected]>
Subject: Re: [PATCH 1/9] improve the layout of the help output
To: William Jon McCann <[email protected]>
Cc: [email protected]
Message-ID:
<[email protected]>
Content-Type: text/plain; charset=UTF-8
Hi,
> The uppercase is because I was trying to be consistent with GOption.
> The later is not because I erred on the side of fewer changes.
> Probably makes sense to capitalize both and possibly even SUBCOMMAND -
> or maybe even replace subcommand with COMMAND (a la git).
Sounds fine to me. Just as long as it's consistent.
> Sure that's fine too. Maybe even "Options for %s command:" ?
Yup, sounds good (assuming you make the s/subcommand/COMMAND/ mentioned above).
--Ray
------------------------------
Message: 3
Date: Mon, 23 Feb 2009 16:52:29 -0500
From: Ray Strode <[email protected]>
Subject: Re: [PATCH 7/9] add a message display method
To: [email protected]
Cc: [email protected], William Jon McCann
<[email protected]>
Message-ID:
<[email protected]>
Content-Type: text/plain; charset=UTF-8
Hi,
On Mon, Feb 23, 2009 at 3:35 PM, <[email protected]> wrote:
> From: William Jon McCann <[email protected]>
>
> Useful for showing messages like "Shutting down..." etc. A plugin
> may choose not to support this feature.
Sounds good. I do have some vague recollection of Charlie already
adding something like this, but maybe I'm crazy.
Charlie, you reading this? Am I crazy?
> diff --git a/src/client/plymouth.c b/src/client/plymouth.c
> index 60e86fb..fd8f948 100644
> --- a/src/client/plymouth.c
> +++ b/src/client/plymouth.c
[...]
> @@ -619,6 +619,7 @@ main (int argc,
> "ask-for-password", "Ask user for
> password", PLY_COMMAND_OPTION_TYPE_FLAG,
> "ignore-keystroke", "Remove sensitivity to
> a keystroke", PLY_COMMAND_OPTION_TYPE_STRING,
> "update", "Tell boot daemon an update about
> boot progress", PLY_COMMAND_OPTION_TYPE_STRING,
> + "message", "Tell boot daemon to display a
> message", PLY_COMMAND_OPTION_TYPE_STRING,
> "details", "Tell boot daemon there were
> errors during boot", PLY_COMMAND_OPTION_TYPE_FLAG,
> "wait", "Wait for boot daemon to quit",
> PLY_COMMAND_OPTION_TYPE_FLAG,
> NULL);
> @@ -706,6 +707,7 @@ main (int argc,
> "sysinit", &should_sysinit,
> "show-splash", &should_show_splash,
> "hide-splash", &should_hide_splash,
> + "message", &message,
> "ask-for-password",
> &should_ask_for_password,
> "ignore-keystroke", &ignore_keystroke,
> "update", &status,
So, when I first wrote the client all command line arguments were of the form:
plymouth --option
Kristian convinced me to go to the git form of
plymouth command --option
But I did a half-assed job, and there are still some --option types
there for compatibility with the scripts calling it. I do sort of
think that all new code should add commands instead of options though.
[...]
> --- a/src/plugins/splash/text/plugin.c
> +++ b/src/plugins/splash/text/plugin.c
[...]
> @@ -107,17 +110,45 @@ destroy_plugin (ply_boot_splash_plugin_t *plugin)
> hide_splash_screen (plugin, plugin->loop);
>
> ply_text_progress_bar_free (plugin->progress_bar);
> + if (plugin->message != NULL)
> + free (plugin->message);
free allows NULL, so ditch the check.
[...]
> static void
> +show_message (ply_boot_splash_plugin_t *plugin)
> +{
> + int window_width, window_height;
> + int i;
> +
> + window_width = ply_window_get_number_of_text_columns (plugin->window);
> + window_height = ply_window_get_number_of_text_rows (plugin->window);
> +
> + ply_window_set_text_cursor_position (plugin->window,
> + 0, window_height / 2);
> +
> + for (i=0; i < window_width; i++)
> + {
> + write (STDOUT_FILENO, " ", strlen (" "));
> + }
Are you trying to clear the line here? There is a helper function for that.
I think it's ply_window_clear_line_text or something.
Rest looks good.
--Ray
------------------------------
Message: 4
Date: Mon, 23 Feb 2009 16:55:31 -0500
From: Ray Strode <[email protected]>
Subject: Re: [PATCH 8/9] add debug command line option
To: [email protected]
Cc: [email protected], William Jon McCann
<[email protected]>
Message-ID:
<[email protected]>
Content-Type: text/plain; charset=UTF-8
Hi,
On Mon, Feb 23, 2009 at 3:35 PM, <[email protected]> wrote:
> From: William Jon McCann <[email protected]>
>
> Used to override the kernel command line and enabled debugging
Makes sense.
[...]
> + if (debug)
> + ply_toggle_tracing ();
> +
Should be:
if (debug && !ply_is_tracing ())
otherwise it will do the wrong thing if debugging is turned on from
the kernel command line.
I thought I already had this, but maybe I only did it in the client or
did it on a throwaway branch or something.
--Ray
------------------------------
Message: 5
Date: Mon, 23 Feb 2009 17:12:05 -0500
From: Ray Strode <[email protected]>
Subject: Re: [PATCH 9/9] add way to detect if an option was set or not
To: [email protected]
Cc: [email protected], William Jon McCann
<[email protected]>
Message-ID:
<[email protected]>
Content-Type: text/plain; charset=UTF-8
Hi,
On Mon, Feb 23, 2009 at 3:35 PM, <[email protected]> wrote:
> From: William Jon McCann <[email protected]>
>
May be add a description like:
With integer options there's no obvious value to mean "unset" like there is for
string types (and to a lesser extent boolean types), so this commit
adds a mechanism
to do that.
> --- a/src/client/plymouth.c
> +++ b/src/client/plymouth.c
> @@ -699,20 +699,20 @@ main (int argc,
> }
>
> ply_command_parser_get_options (state.command_parser,
> - "help", &should_help,
> - "debug", &should_be_verbose,
> - "newroot", &chroot_dir,
> - "quit", &should_quit,
> - "ping", &should_ping,
> - "sysinit", &should_sysinit,
> - "show-splash", &should_show_splash,
> - "hide-splash", &should_hide_splash,
> - "message", &message,
> - "ask-for-password",
> &should_ask_for_password,
> - "ignore-keystroke", &ignore_keystroke,
> - "update", &status,
> - "wait", &should_wait,
> - "details", &report_error,
> + "help", &should_help, NULL,
> + "debug", &should_be_verbose, NULL,
> + "newroot", &chroot_dir, NULL,
> + "quit", &should_quit, NULL,
> + "ping", &should_ping, NULL,
> + "sysinit", &should_sysinit, NULL,
> + "show-splash", &should_show_splash, NULL,
> + "hide-splash", &should_hide_splash, NULL,
> + "message", &message, NULL,
> + "ask-for-password",
> &should_ask_for_password, NULL,
> + "ignore-keystroke", &ignore_keystroke,
> NULL,
> + "update", &status, NULL,
> + "wait", &should_wait, NULL,
> + "details", &report_error, NULL,
> NULL);
I'm not a huge fan of this interface. It makes a long function call
even longer, and the "is it set?" argument will rarely be used.
I'd rather separate:
ply_command_parser_is_option_set and ply_command_parser_is_command_option_set
functions, or alternatively:
ply_command_parser_get_option (parser, "name", &value, &is_set);
and
ply_command_parser_get_command_option (parser, "command", "name",
&value, &is_set);
that only takes one name and has the new argument. Then you could
just do the atypical cases separately.
[...]
> --- a/src/libply/ply-command-parser.c
> +++ b/src/libply/ply-command-parser.c
> @@ -46,7 +46,7 @@ typedef struct
> char *name;
> char *description;
> ply_command_option_type_t type;
> -
> + bool was_set;
> ply_command_option_result_t result;
> } ply_command_option_t;
no bool in structs please. Instead:
uint32_t was_set : 1;
It packs better that way.
--Ray
------------------------------
Message: 6
Date: Tue, 24 Feb 2009 11:26:23 +0000
From: Charlie Brej <[email protected]>
Subject: Re: [PATCH 5/9] add a mode commandline option
To: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
[email protected] wrote:
> Initially supports modes: boot, shutdown. This allows the
> progress cache to be loaded from the appropriate file.
A couple weeks back I had a bit of a hack around with the idea of showing
plymouth on shutdown and although I could get it to do its stuff I was a bit
scared of starting the splash before X had finished. Also I wasn't sure about
the sequence of unmounting disks and the killall and trying to shutdown
plymouth
before it gets killed. Might be easiest to let plymouth be killed and let the
shutdown duration file not be updated. It's probably all fine and I'm just
fretting. Thanks for doing this.
Do we want the splashes to be aware that they are in shutdown mode? What would
they do different?
In main, state.mode is not explicitly set to a default (PLY_MODE_BOOT is 0 so
it
works anyway).
Ray Strode wrote:
>no bool in structs please. Instead:
>uint32_t was_set : 1;
>It packs better that way.
Just wondering, what happens when you ask for a pointer to a packed value?
With all patches applied I am getting segfaults. I may have skipped one of the
patches so if you're not getting them then ignore and I will re-check once it's
in git.
[r...@localhost plymouth]# valgrind plymouth quit
==26165== Memcheck, a memory error detector.
==26165== Copyright (C) 2002-2007, and GNU GPL'd, by Julian Seward et al.
==26165== Using LibVEX rev 1804, a library for dynamic binary translation.
==26165== Copyright (C) 2004-2007, and GNU GPL'd, by OpenWorks LLP.
==26165== Using valgrind-3.3.0, a dynamic binary instrumentation framework.
==26165== Copyright (C) 2000-2007, and GNU GPL'd, by Julian Seward et al.
==26165== For more details, rerun with: -v
==26165==
==26165== Conditional jump or move depends on uninitialised value(s)
==26165== at 0x402795C: ply_command_parser_get_options_for_command
(ply-command-parser.c:482)
==26165== by 0x40281A5: ply_command_parser_get_command_options
(ply-command-parser.c:583)
==26165== by 0x804B796: on_quit_request (plymouth.c:580)
==26165== by 0x4028085: on_command_dispatch_timeout
(ply-command-parser.c:842)
==26165== by 0x4026393: ply_event_loop_process_pending_events
(ply-event-loop.c:1145)
==26165== by 0x4026877: ply_event_loop_run (ply-event-loop.c:1251)
==26165== by 0x804AD8A: main (plymouth.c:839)
==26165==
==26165== Use of uninitialised value of size 4
==26165== at 0x40074FB: strcmp (mc_replace_strmem.c:337)
==26165== by 0x40278DA: ply_command_get_option (ply-command-parser.c:321)
==26165== by 0x4027968: ply_command_parser_get_options_for_command
(ply-command-parser.c:486)
==26165== by 0x40281A5: ply_command_parser_get_command_options
(ply-command-parser.c:583)
==26165== by 0x804B796: on_quit_request (plymouth.c:580)
==26165== by 0x4028085: on_command_dispatch_timeout
(ply-command-parser.c:842)
==26165== by 0x4026393: ply_event_loop_process_pending_events
(ply-event-loop.c:1145)
==26165== by 0x4026877: ply_event_loop_run (ply-event-loop.c:1251)
==26165== by 0x804AD8A: main (plymouth.c:839)
==26165==
==26165== Invalid read of size 1
==26165== at 0x40074FB: strcmp (mc_replace_strmem.c:337)
==26165== by 0x40278DA: ply_command_get_option (ply-command-parser.c:321)
==26165== by 0x4027968: ply_command_parser_get_options_for_command
(ply-command-parser.c:486)
==26165== by 0x40281A5: ply_command_parser_get_command_options
(ply-command-parser.c:583)
==26165== by 0x804B796: on_quit_request (plymouth.c:580)
==26165== by 0x4028085: on_command_dispatch_timeout
(ply-command-parser.c:842)
==26165== by 0x4026393: ply_event_loop_process_pending_events
(ply-event-loop.c:1145)
==26165== by 0x4026877: ply_event_loop_run (ply-event-loop.c:1251)
==26165== by 0x804AD8A: main (plymouth.c:839)
==26165== Address 0x3f847ae1 is not stack'd, malloc'd or (recently) free'd
==26165==
==26165== Process terminating with default action of signal 11 (SIGSEGV)
==26165== Access not within mapped region at address 0x3F847AE1
==26165== at 0x40074FB: strcmp (mc_replace_strmem.c:337)
==26165== by 0x40278DA: ply_command_get_option (ply-command-parser.c:321)
==26165== by 0x4027968: ply_command_parser_get_options_for_command
(ply-command-parser.c:486)
==26165== by 0x40281A5: ply_command_parser_get_command_options
(ply-command-parser.c:583)
==26165== by 0x804B796: on_quit_request (plymouth.c:580)
==26165== by 0x4028085: on_command_dispatch_timeout
(ply-command-parser.c:842)
==26165== by 0x4026393: ply_event_loop_process_pending_events
(ply-event-loop.c:1145)
==26165== by 0x4026877: ply_event_loop_run (ply-event-loop.c:1251)
==26165== by 0x804AD8A: main (plymouth.c:839)
==26165==
==26165== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 18 from 1)
==26165== malloc/free: in use at exit: 3,118 bytes in 173 blocks.
==26165== malloc/free: 175 allocs, 2 frees, 3,240 bytes allocated.
==26165== For counts of detected errors, rerun with: -v
==26165== searching for pointers to 173 not-freed blocks.
==26165== checked 84,780 bytes.
==26165==
==26165== LEAK SUMMARY:
==26165== definitely lost: 0 bytes in 0 blocks.
==26165== possibly lost: 0 bytes in 0 blocks.
==26165== still reachable: 3,118 bytes in 173 blocks.
==26165== suppressed: 0 bytes in 0 blocks.
==26165== Rerun with --leak-check=full to see details of leaked memory.
Segmentation fault
[r...@localhost plymouth]# valgrind plymouth ask-question
==26166== Memcheck, a memory error detector.
==26166== Copyright (C) 2002-2007, and GNU GPL'd, by Julian Seward et al.
==26166== Using LibVEX rev 1804, a library for dynamic binary translation.
==26166== Copyright (C) 2004-2007, and GNU GPL'd, by OpenWorks LLP.
==26166== Using valgrind-3.3.0, a dynamic binary instrumentation framework.
==26166== Copyright (C) 2000-2007, and GNU GPL'd, by Julian Seward et al.
==26166== For more details, rerun with: -v
==26166==
==26166==
==26166== Process terminating with default action of signal 11 (SIGSEGV)
==26166== Bad permissions for mapped region at address 0x804C6C0
==26166== at 0x4027956: ply_command_parser_get_options_for_command
(ply-command-parser.c:539)
==26166== by 0x40281A5: ply_command_parser_get_command_options
(ply-command-parser.c:583)
==26166== by 0x804B944: on_question_request (plymouth.c:480)
==26166== by 0x4028085: on_command_dispatch_timeout
(ply-command-parser.c:842)
==26166== by 0x4026393: ply_event_loop_process_pending_events
(ply-event-loop.c:1145)
==26166== by 0x4026877: ply_event_loop_run (ply-event-loop.c:1251)
==26166== by 0x804AD8A: main (plymouth.c:839)
==26166==
==26166== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 18 from 1)
==26166== malloc/free: in use at exit: 3,118 bytes in 173 blocks.
==26166== malloc/free: 175 allocs, 2 frees, 3,240 bytes allocated.
==26166== For counts of detected errors, rerun with: -v
==26166== searching for pointers to 173 not-freed blocks.
==26166== checked 84,784 bytes.
==26166==
==26166== LEAK SUMMARY:
==26166== definitely lost: 0 bytes in 0 blocks.
==26166== possibly lost: 0 bytes in 0 blocks.
==26166== still reachable: 3,118 bytes in 173 blocks.
==26166== suppressed: 0 bytes in 0 blocks.
==26166== Rerun with --leak-check=full to see details of leaked memory.
Segmentation fault
------------------------------
Message: 7
Date: Tue, 24 Feb 2009 09:44:43 -0500
From: William Jon McCann <[email protected]>
Subject: Re: [PATCH 5/9] add a mode commandline option
To: Charlie Brej <[email protected]>
Cc: [email protected]
Message-ID:
<[email protected]>
Content-Type: text/plain; charset=ISO-8859-1
Hi,
On Tue, Feb 24, 2009 at 6:26 AM, Charlie Brej <[email protected]> wrote:
...
> Do we want the splashes to be aware that they are in shutdown mode? What would
> they do different?
The plugins aren't aware of the mode at the moment.
...
> With all patches applied I am getting segfaults. I may have skipped one of the
> patches so if you're not getting them then ignore and I will re-check once
> it's
> in git.
Likely due to the "add a way to see if arg was set" patch being
incomplete. We're going to do that patch differently anyway as per
Ray's review.
But this is worth testing again after the reviewed patches are pushed.
Thanks,
Jon
------------------------------
_______________________________________________
plymouth mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/plymouth
End of plymouth Digest, Vol 5, Issue 4
**************************************
