Re: [ccache] [PATCH v4] add support for '@' parameters
Changes from patch set 3:
- args_insert() now consumes the arguments to be inserted, the caller doesn't
need to subsequently free it. Avoids some strdups
- test cases and args_insert use in ccache.c updated to reflect new semantics
Andrew
> -Original Message-
> From: Boie, Andrew P
> Sent: Monday, July 30, 2012 5:51 PM
> To: ccache@lists.samba.org
> Cc: Boie, Andrew P
> Subject: [PATCH v4] add support for '@' parameters
>
> From: "Boie, Andrew P"
>
> These indicate to the compiler that additional command line options
> should be read from a text file. If encountered, read the file,
> tokenize any arguments, and if any are found, do an in-place replacement
> of the '@' parameter with the arguments within the file.
>
> args_insert() added to insert a set of arguments into a position within
> another set of arguments.
>
> args_init_from_string() has been improved so that any character may be
> included by prefixing that character with a backslash, and support for
> quoted arguments which pass special characters within the quotation marks
> unmodified.
>
> Signed-off-by: Andrew Boie
> ---
> args.c | 115
> +-
> ccache.c | 24 +++-
> ccache.h |1 +
> test/test_args.c | 53 ++---
> 4 files changed, 178 insertions(+), 15 deletions(-)
>
> diff --git a/args.c b/args.c
> index 13a3d37..aa0e5ac 100644
> --- a/args.c
> +++ b/args.c
> @@ -37,17 +37,57 @@ struct args *
> args_init_from_string(const char *command)
> {
> struct args *args;
> - char *p = x_strdup(command);
> - char *q = p;
> - char *word, *saveptr = NULL;
> -
> + const char *pos = command;
> + char *argbuf = x_malloc(strlen(command) + 1);
> + char *argpos = argbuf;
> args = args_init(0, NULL);
> - while ((word = strtok_r(q, " \t\r\n", &saveptr))) {
> - args_add(args, word);
> - q = NULL;
> - }
> + argpos = argbuf;
> + char quoting = '\0';
> +
> + while (1) {
> + switch (*pos) {
> + case '\\':
> + pos++;
> + if (*pos == '\0')
> + continue;
> + break;
>
> - free(p);
> + case '\"': case '\'':
> + if (quoting != '\0') {
> + if (quoting == *pos) {
> + quoting = '\0';
> + pos++;
> + continue;
> + } else
> + break;
> + } else {
> + quoting = *pos;
> + pos++;
> + continue;
> + }
> + case '\n': case '\t': case ' ':
> + if (quoting)
> + break;
> + /* Fall through */
> + case '\0':
> + /* end of token */
> + *argpos = '\0';
> + if (argbuf[0] != '\0')
> + args_add(args, argbuf);
> + argpos = argbuf;
> + if (*pos == '\0')
> + goto out;
> + else {
> + pos++;
> + continue;
> + }
> + }
> + *argpos = *pos;
> + pos++;
> + argpos++;
> + }
> +out:
> + free(argbuf);
> return args;
> }
>
> @@ -57,6 +97,63 @@ args_copy(struct args *args)
> return args_init(args->argc, args->argv);
> }
>
> +/* Insert all arguments in src into dest at position index.
> + * If replace is true, the element at dest->argv[index] is replaced
> + * with the contents of src and everything past it is shifted.
> + * Otherwise, dest->argv[index] is also shifted.
> + *
> + * src is consumed by this operation and should not be freed or used
> + * again by the caller */
> +void
> +args_insert(struct args *dest, int index, struct args *src, bool replace)
> +{
> + int offset;
> + int j;
> +
> + /* Adjustments made if we are replacing or shifting the element
> + * currently at dest->argv[index] */
> + offset = replace ? 1 : 0;
> +
Re: [ccache] [PATCH v3] add support for '@' parameters
> -Original Message- > From: Jürgen Buchmüller [mailto:pullm...@t-online.de] > Sent: Monday, July 30, 2012 4:46 PM > To: Boie, Andrew P > Cc: ccache@lists.samba.org > Subject: Re: [ccache] [PATCH v3] add support for '@' parameters > > Am Montag, den 30.07.2012, 16:05 -0700 schrieb Andrew Boie: > [..snip..] > > + /* Trivial case; replace with 1 element */ > > + dest->argv[index] = x_strdup(src->argv[0]); > [..snap..] > > Shouldn't dest->argv[index] be free()d before overwriting it? This was already freed, look 7 lines up. > > + /* Copy the new arguments into place */ > > + for (j = 0; j < src->argc; j++) > > + dest->argv[j + index] = x_strdup(src->argv[j]); > > Shouldn't src->argv[j] be free()d after it is strdup()ed? > Or alternatively: why not copy src->argv[j] an re-use it? src array isn't touched by this function the way it is currently implemented; this is stated in the function comments. I could change this to have the function consume src if desired; I'm not sure what other cases Joel had in mind for this function when he asked me to move it to common code. Andrew ___ ccache mailing list ccache@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache
Re: [ccache] [PATCH v2] add support for '@' parameters
This was sent in error, please ignore. Patch isn't quite ready yet.
Andrew
> -Original Message-
> From: Boie, Andrew P
> Sent: Monday, July 30, 2012 3:38 PM
> To: ccache@lists.samba.org
> Cc: Boie, Andrew P
> Subject: [PATCH v2] add support for '@' parameters
>
> From: "Boie, Andrew P"
>
> These indicate to the compiler that additional command line options
> should be read from a text file. If encountered, read the file,
> tokenize any arguments, and if any are found, do an in-place replacement
> of the '@' parameter with the arguments within the file.
>
> args_insert() added to insert a set of arguments into a position within
> another set of arguments.
>
> args_init_from_string() has been improved so that any character may be
> included by prefixing that character with a backslash, and support for
> quoted arguments which pass special characters within the quotation marks
> unmodified.
>
> Signed-off-by: Andrew Boie
> ---
> args.c | 108
> +-
> ccache.c | 25 -
> ccache.h |1 +
> test/test_args.c | 47 +---
> 4 files changed, 166 insertions(+), 15 deletions(-)
>
> diff --git a/args.c b/args.c
> index 13a3d37..ee84951 100644
> --- a/args.c
> +++ b/args.c
> @@ -37,17 +37,57 @@ struct args *
> args_init_from_string(const char *command)
> {
> struct args *args;
> - char *p = x_strdup(command);
> - char *q = p;
> - char *word, *saveptr = NULL;
> -
> + const char *pos = command;
> + char *argbuf = x_malloc(strlen(command) + 1);
> + char *argpos = argbuf;
> args = args_init(0, NULL);
> - while ((word = strtok_r(q, " \t\r\n", &saveptr))) {
> - args_add(args, word);
> - q = NULL;
> - }
> + argpos = argbuf;
> + char quoting = '\0';
> +
> + while (1) {
> + switch (*pos) {
> + case '\\':
> + pos++;
> + if (*pos == '\0')
> + continue;
> + break;
>
> - free(p);
> + case '\"': case '\'':
> + if (quoting != '\0') {
> + if (quoting == *pos) {
> + quoting = '\0';
> + pos++;
> + continue;
> + } else
> + break;
> + } else {
> + quoting = *pos;
> + pos++;
> + continue;
> + }
> + case '\n': case '\t': case ' ':
> + if (quoting)
> + break;
> + /* Fall through */
> + case '\0':
> + /* end of token */
> + *argpos = '\0';
> + if (argbuf[0] != '\0')
> + args_add(args, argbuf);
> + argpos = argbuf;
> + if (*pos == '\0')
> + goto out;
> + else {
> + pos++;
> + continue;
> + }
> + }
> + *argpos = *pos;
> + pos++;
> + argpos++;
> + }
> +out:
> + free(argbuf);
> return args;
> }
>
> @@ -57,6 +97,56 @@ args_copy(struct args *args)
> return args_init(args->argc, args->argv);
> }
>
> +/* Insert all arguments in src into dest at position index.
> + * If replace is true, the element at dest->argv[index] is replaced
> + * with the contents of src and everything past it is shifted.
> + * Otherwise, dest->argv[index] is also shifted.
> + *
> + * This operation modifies dest but leaves src untouched. */
> +void
> +args_insert(struct args *dest, int index, struct args *src, bool replace)
> +{
> + int offset;
> + int j;
> +
> + /* Adjustments made if we are replacing or shifting the element
> + * currently at dest->argv[index] */
> + offset = replace ? 1 : 0;
> +
> + if (replace)
> + free(dest->argv[index]);
> +
> + if (src->argc == 0) {
> + if (replace) {
> + /* Have to shift everything down
Re: [ccache] [PATCH] add support for '@' parameters
> -Original Message- > From: joel.rosd...@gmail.com [mailto:joel.rosd...@gmail.com] On Behalf Of > Joel Rosdahl > > + file_args = args_init_from_string(argdata); > > Isn't args_init_from_string too simplistic for parsing the contents? Oof, yeah you're right. It seems that args_init_from_string needs to be significantly beefed-up. I don't suppose you know of some library code somewhere that does this? I should dig into the bash sources, IIRC it's the shell that turns the string into argv array... > It would be nice if this code could be extracted to a new > args_insert(struct args *args, struct args *to_insert, int pos) > function or so. (There's a test suite for args_* in test/test_args.c.) Done. I'll add a test case as well. -Andrew ___ ccache mailing list ccache@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache
[ccache] GCC '@' parameters
I've been playing around with using the 3.x series of ccache to speed up Android builds (they by default use an old 2.x prebuilt copy) and have run into an incompatibility where I get 'unsupported compiler option' when the @file parameter is used. Android started using this extensively in Jelly Bean. >From the documentation in >http://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html: @file Read command-line options from file. The options read are inserted in place of the original @file option. If file does not exist, or cannot be read, then the option will be treated literally, and not removed. Options in file are separated by whitespace. A whitespace character may be included in an option by surrounding the entire option in either single or double quotes. Any character (including a backslash) may be included by prefixing the character to be included with a backslash. The file may itself contain additional @file options; any such options will be processed recursively. However any invocation of ccache where this is used in the GCC command line results in "Unsupported Compiler Option" with ccache 3.x: [2012-07-11T12:02:49.208752 11127] Compiler option @out/target/product/full_x86/obj/STATIC_LIBRARIES/libv8_intermediates/import_includes is unsupported ccache 2.x seems OK with this, but is probably computing incorrect command line hashes since it's not diving into the file to see the command line options within. Any advice for getting around this? Regards, Andrew Boie ___ ccache mailing list ccache@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache
