Re: [ccache] [PATCH] add support for '@' parameters

2012-07-30 Thread Boie, Andrew P
 -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


Re: [ccache] [PATCH v3] add support for '@' parameters

2012-07-30 Thread Boie, Andrew P


 -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 v4] add support for '@' parameters

2012-07-30 Thread Boie, Andrew P
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 andrew.p.b...@intel.com
 
 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 andrew.p.b...@intel.com
 ---
  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;
 +
 + if (replace)
 + free(dest-argv[index]);
 +
 + if (src-argc == 0) {
 + if (replace) {
 + /* Have to shift everything down by 1 since
 +  * we replaced with an empty list */
 + for (j = index; j  dest-argc; j++)
 + dest-argv[j] = dest-argv[j + 1];
 + dest-argc--;
 + }
 + args_free(src);
 + return;
 + }
 +
 + if (src-argc == 1  replace) {
 + /* Trivial case; replace with 1 element */
 + dest-argv[index] = src-argv[0];
 + src-argc = 0

[ccache] GCC '@' parameters

2012-07-11 Thread Boie, Andrew P
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