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. [PATCH 9/9] add way to detect if an option was set or not
([email protected])
2. Re: [PATCH 1/9] improve the layout of the help output (Ray Strode)
3. Re: [PATCH 2/9] allow the progress cache file to be
configurable (Ray Strode)
4. Re: [PATCH 3/9] add support for long data types (Ray Strode)
5. Re: [PATCH 5/9] add a mode commandline option (Ray Strode)
6. Re: [PATCH 4/9] add command line parsing to daemon (Ray Strode)
7. Re: [PATCH 1/9] improve the layout of the help output
(William Jon McCann)
8. Re: [PATCH 6/9] add no-daemon command line option (Ray Strode)
----------------------------------------------------------------------
Message: 1
Date: Mon, 23 Feb 2009 15:35:57 -0500
From: [email protected]
Subject: [PATCH 9/9] add way to detect if an option was set or not
To: [email protected]
Cc: William Jon McCann <[email protected]>
Message-ID:
<[email protected]>
From: William Jon McCann <[email protected]>
---
src/client/plymouth.c | 28 ++++++++++++++--------------
src/libply/ply-command-parser.c | 16 ++++++++++++----
src/libply/ply-command-parser.h | 3 ++-
src/main.c | 17 +++++++++++------
4 files changed, 39 insertions(+), 25 deletions(-)
diff --git a/src/client/plymouth.c b/src/client/plymouth.c
index fd8f948..3463498 100644
--- 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);
if (should_help || argc < 2)
diff --git a/src/libply/ply-command-parser.c b/src/libply/ply-command-parser.c
index 5d77e00..1427e95 100644
--- 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;
@@ -473,6 +473,7 @@ ply_command_parser_get_options_for_command
(ply_command_parser_t *parser,
const char *option_name,
va_list args)
{
+ bool *option_was_set;
assert (parser != NULL);
assert (command != NULL);
@@ -533,6 +534,10 @@ ply_command_parser_get_options_for_command
(ply_command_parser_t *parser,
}
}
+ option_was_set = va_arg (args, bool *);
+ if (option_was_set != NULL)
+ *option_was_set = option->was_set;
+
option_name = va_arg (args, const char *);
}
}
@@ -540,7 +545,8 @@ ply_command_parser_get_options_for_command
(ply_command_parser_t *parser,
void
ply_command_parser_get_options (ply_command_parser_t *parser,
const char *option_name, /*
- void * option_result */
+ void * option_result,
+ bool * option_was_set */
...)
{
va_list args;
@@ -557,7 +563,8 @@ void
ply_command_parser_get_command_options (ply_command_parser_t *parser,
const char *command_name,
const char *option_name, /*
- void * option_result */
+ void * option_result,
+ bool * option_was_set */
...)
{
ply_command_t *command;
@@ -763,7 +770,8 @@ ply_command_read_option (ply_command_t *command,
ply_list_remove_node (arguments, node);
- ply_command_option_read_arguments (option, arguments);
+ if (ply_command_option_read_arguments (option, arguments))
+ option->was_set = true;
return option;
}
diff --git a/src/libply/ply-command-parser.h b/src/libply/ply-command-parser.h
index 805b518..446f87a 100644
--- a/src/libply/ply-command-parser.h
+++ b/src/libply/ply-command-parser.h
@@ -64,7 +64,8 @@ void ply_command_parser_get_options (ply_command_parser_t
*parser,
void ply_command_parser_get_command_options (ply_command_parser_t *parser,
const char *command_name,
const char *option_name, /*
- void * option_result */
+ void * option_result,
+ bool * option_was_set */
...);
void ply_command_parser_stop_parsing_arguments (ply_command_parser_t *parser);
diff --git a/src/main.c b/src/main.c
index ce482ad..eacde88 100644
--- a/src/main.c
+++ b/src/main.c
@@ -1154,6 +1154,8 @@ main (int argc,
bool should_help = false;
bool no_daemon = false;
bool debug = false;
+ bool was_set = false;
+ long attach_to_session;
ply_daemon_handle_t *daemon_handle;
char *mode_string = NULL;
@@ -1182,11 +1184,11 @@ main (int argc,
}
ply_command_parser_get_options (state.command_parser,
- "help", &should_help,
- "mode", &mode_string,
- "no-daemon", &no_daemon,
- "debug", &debug,
- "attach-to-session", &state.ptmx,
+ "help", &should_help, NULL,
+ "mode", &mode_string, NULL,
+ "no-daemon", &no_daemon, NULL,
+ "debug", &debug, NULL,
+ "attach-to-session", &attach_to_session,
&was_set,
NULL);
if (should_help)
{
@@ -1216,6 +1218,9 @@ main (int argc,
free (mode_string);
}
+ if (was_set)
+ state.ptmx = attach_to_session;
+
if (geteuid () != 0)
{
ply_error ("plymouthd must be run as root user");
@@ -1259,7 +1264,7 @@ main (int argc,
state.boot_buffer = ply_buffer_new ();
- if (state.ptmx != 0)
+ if (state.ptmx != -1)
{
if (!attach_to_running_session (&state))
{
--
1.6.1.3
------------------------------
Message: 2
Date: Mon, 23 Feb 2009 16:07:05 -0500
From: Ray Strode <[email protected]>
Subject: Re: [PATCH 1/9] improve the layout of the help output
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]>
>
> Left align descriptions and group subcommands.
Ah, this looks a lot better.
> --- a/src/libply/ply-command-parser.c
> +++ b/src/libply/ply-command-parser.c
> @@ -139,26 +139,37 @@ append_usage_line_to_buffer (ply_command_parser_t
> *parser,
>
> ply_buffer_append (buffer, "%s\n",
> parser->main_command->description);
> - ply_buffer_append (buffer, "USAGE: %s", parser->main_command->name);
> + ply_buffer_append (buffer, "USAGE: %s [OPTION...]",
> parser->main_command->name);
>
> - option_node = ply_list_get_first_node (parser->main_command->options);
> - while (option_node != NULL)
> - {
> - ply_command_option_t *option;
> + if (ply_list_get_length (parser->available_subcommands) > 0)
> + ply_buffer_append (buffer, " [subcommand [options]...]\n");
> +}
Why is OPTION uppercase but subcommand and options are lowercase?
[...]
> + command_node = ply_list_get_first_node (parser->available_subcommands);
> + while (command_node != NULL)
> + {
> + ply_command_t *command;
> +
> + command = (ply_command_t *) ply_list_node_get_data (command_node);
> +
> + if (ply_list_get_first_node (command->options) != NULL)
> + {
> + ply_buffer_append (buffer, "\nSubcommand Options (%s):\n",
> command->name);
> +
> + append_command_options_to_buffer (parser, command, buffer);
> + }
The "Subcommand Options (%s)" looks a little repetitive in the output.
Maybe it would be better if it were more sentence-y, like:
"Options for ask-for-password subcommand:"
I'm don't really care either way, but if you feel your proposed way is
better than the above, you should fix the inconsistency where
"Subcommand Options" is titlecase and "Available subcommands" isn't.
Feel free to push this one.
--Ray
------------------------------
Message: 3
Date: Mon, 23 Feb 2009 16:08:45 -0500
From: Ray Strode <[email protected]>
Subject: Re: [PATCH 2/9] allow the progress cache file to be
configurable
To: [email protected]
Cc: [email protected], William Jon McCann
<[email protected]>
Message-ID:
<[email protected]>
Content-Type: text/plain; charset=UTF-8
Hey,
On Mon, Feb 23, 2009 at 3:35 PM, <[email protected]> wrote:
> From: William Jon McCann <[email protected]>
>
> This will enable using separate cache files for different plymouth modes.
Looks good.
--Ray
------------------------------
Message: 4
Date: Mon, 23 Feb 2009 16:15:41 -0500
From: Ray Strode <[email protected]>
Subject: Re: [PATCH 3/9] add support for long data types
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]>
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.
> ---
> src/libply/ply-command-parser.c | 37 +++++++++++++++++++++++++++++++++++++
> src/libply/ply-command-parser.h | 3 ++-
> 2 files changed, 39 insertions(+), 1 deletions(-)
>
> diff --git a/src/libply/ply-command-parser.c b/src/libply/ply-command-parser.c
> index 29d47fa..5d77e00 100644
> --- a/src/libply/ply-command-parser.c
> +++ b/src/libply/ply-command-parser.c
> @@ -38,6 +38,7 @@ typedef union
> bool as_boolean;
> char *as_string;
> int as_integer;
> + int as_long;
> } ply_command_option_result_t;
Presumably, the "int as_long" should be "long as_long", no?
The rest of the patch looks okay.
--Ray
------------------------------
Message: 5
Date: Mon, 23 Feb 2009 16:29:18 -0500
From: Ray Strode <[email protected]>
Subject: Re: [PATCH 5/9] add a mode commandline 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]>
>
> Initially supports modes: boot, shutdown. This allows the
> progress cache to be loaded from the appropriate file.
Makes sense.
> +enum {
> + PLY_MODE_BOOT,
> + PLY_MODE_SHUTDOWN
> +} type;
This should be a named type... something like
typedef enum { ... } ply_mode_t;
>
> typedef struct
> {
> @@ -81,6 +87,7 @@ typedef struct
> ply_buffer_t *entry_buffer;
> ply_command_parser_t *command_parser;
> long ptmx;
> + int mode;
And this should use the above type instead of int.
[...]
> +static const char *
> +get_cache_file_for_mode (int mode)
here, too.
[...]
> +static const char *
> +get_log_file_for_mode (int mode)
> +{
> + const char *filename;
> +
> + switch (mode)
> + {
> + case PLY_MODE_BOOT:
> + filename = PLYMOUTH_LOG_DIRECTORY "/boot.log";
> + break;
> + case PLY_MODE_SHUTDOWN:
> + filename = PLYMOUTH_LOG_DIRECTORY "/shutdown.log";
> + break;
I don't think it makes much sense to have a shutdown.log. boot.log
currently only shows
the boot messages of the currently running system. shutdown.log would
have to be
written out half way though shutdown (since the disks get
unmounted...) and it's not clear
what it's lifetime would be. We also don't have a viewer for it like
we do for boot.log. So,
I'd say it's better to just drop it.
[...]
> +static const char *
> +get_log_spool_file_for_mode (int mode)
> +{
> + const char *filename;
> +
> + switch (mode)
> + {
> + case PLY_MODE_BOOT:
> + filename = PLYMOUTH_SPOOL_DIRECTORY "/boot.log";
> + break;
> + case PLY_MODE_SHUTDOWN:
> + filename = PLYMOUTH_SPOOL_DIRECTORY "/shutdown.log";
> + break;
same here...
[...]
> @@ -1055,6 +1153,7 @@ main (int argc,
>
> ply_command_parser_add_options (state.command_parser,
> "help", "This help message",
> PLY_COMMAND_OPTION_TYPE_FLAG,
> + "mode", "Mode is one of: boot, shutdown",
> PLY_COMMAND_OPTION_TYPE_STRING,
> "attach-to-session", "pty_master_fd",
> PLY_COMMAND_OPTION_TYPE_LONG,
> NULL);
Might make sense to have either an PLY_COMMAND_OPTION_TYPE_ENUM or
PLY_COMMAND_OPTION_TYPE_CUSTOM (which took a helper function) to deal
with this. Probably not worth the effort though.
--Ray
------------------------------
Message: 6
Date: Mon, 23 Feb 2009 16:33:36 -0500
From: Ray Strode <[email protected]>
Subject: Re: [PATCH 4/9] add command line parsing to daemon
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]>
>
> If for no other reason than to handle --help.
Right, makes a lot of sense. I've been wanting to do this for a
while, but never made it to the top of my list.
[...]
> --- a/src/main.c
> +++ b/src/main.c
[...]
> + ply_command_parser_add_options (state.command_parser,
> + "help", "This help message",
> PLY_COMMAND_OPTION_TYPE_FLAG,
> + "attach-to-session", "pty_master_fd",
> PLY_COMMAND_OPTION_TYPE_LONG,
> + NULL);
> +
I mentioned this in an earlier thread, but this should be TYPE_INT not TYPE_LONG
Otherwise, looks good! Feel free to push.
--Ray
------------------------------
Message: 7
Date: Mon, 23 Feb 2009 16:34:05 -0500
From: William Jon McCann <[email protected]>
Subject: Re: [PATCH 1/9] improve the layout of the help output
To: Ray Strode <[email protected]>
Cc: [email protected]
Message-ID:
<[email protected]>
Content-Type: text/plain; charset=ISO-8859-1
Hey,
On Mon, Feb 23, 2009 at 4:07 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]>
>>
>> Left align descriptions and group subcommands.
> Ah, this looks a lot better.
>
>> --- a/src/libply/ply-command-parser.c
>> +++ b/src/libply/ply-command-parser.c
>> @@ -139,26 +139,37 @@ append_usage_line_to_buffer (ply_command_parser_t
>> *parser,
>>
>> ply_buffer_append (buffer, "%s\n",
>> parser->main_command->description);
>> - ply_buffer_append (buffer, "USAGE: %s", parser->main_command->name);
>> + ply_buffer_append (buffer, "USAGE: %s [OPTION...]",
>> parser->main_command->name);
>>
>> - option_node = ply_list_get_first_node (parser->main_command->options);
>> - while (option_node != NULL)
>> - {
>> - ply_command_option_t *option;
>> + if (ply_list_get_length (parser->available_subcommands) > 0)
>> + ply_buffer_append (buffer, " [subcommand [options]...]\n");
>> +}
> Why is OPTION uppercase but subcommand and options are lowercase?
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).
> [...]
>> + command_node = ply_list_get_first_node (parser->available_subcommands);
>> + while (command_node != NULL)
>> + {
>> + ply_command_t *command;
>> +
>> + command = (ply_command_t *) ply_list_node_get_data (command_node);
>> +
>> + if (ply_list_get_first_node (command->options) != NULL)
>> + {
>> + ply_buffer_append (buffer, "\nSubcommand Options (%s):\n",
>> command->name);
>> +
>> + append_command_options_to_buffer (parser, command, buffer);
>> + }
> The "Subcommand Options (%s)" looks a little repetitive in the output.
>
> Maybe it would be better if it were more sentence-y, like:
>
> "Options for ask-for-password subcommand:"
>
> I'm don't really care either way, but if you feel your proposed way is
> better than the above, you should fix the inconsistency where
> "Subcommand Options" is titlecase and "Available subcommands" isn't.
Sure that's fine too. Maybe even "Options for %s command:" ?
Thanks,
Jon
------------------------------
Message: 8
Date: Mon, 23 Feb 2009 16:35:20 -0500
From: Ray Strode <[email protected]>
Subject: Re: [PATCH 6/9] add no-daemon 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]>
>
> Useful for debugging and running under gdb
Sounds good to me. Patch looks good.
--Ray
------------------------------
_______________________________________________
plymouth mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/plymouth
End of plymouth Digest, Vol 5, Issue 3
**************************************
