Hold on. I have much improved version almost ready. It passes all the tests now.
2016-09-30 0:15 GMT+02:00 Bruce Korb <bruce.k...@gmail.com>:
> I usually try to catch emails with "fixincludes" in the title.
> Can I please get a copy of the original patch? Thanks.
>
>
> On 09/29/16 11:44, Jeff Law wrote:
>>
>> On 09/22/2016 11:26 PM, Tadek Kijkowski wrote:
>>>
>>> The fixincl executable uses system function to call applyfix or to
>>> direcly patch a header file, with parameters enclosed in single
>>> quotes. This problem is that MinGW system function just calls cmd.exe,
>>> which doesn't strip quotes from parameters and completely ignores
>>> quotes for embedding spaces in parameters. The MinGW system function
>>> also doesn't allow for newlines in executed command parameters. As a
>>> result fixincludes doesn't wotk at on when trying to build a cross
>>> compiler with mingw as host.
>>>
>>> On MinGW host, this patch adds system_with_shell function, which
>>> transforms command passed to system function is following way:
>>> - Enclose entire command in double quotes
>>> - Prepend shell executable name, taken from environment variable SHELL
>>> - Replace each occurence of newline with '$'\n'' sequence which is
>>> understood by bash and ksh (it is assumed that all newlines are
>>> embedded in command parameters enclosed in single quotes)
>>>
>>> 2016-09-23 Tadek Kijkowski (tkijkow...@gmail.com)
>>>
>>> * fixincl.c: Added system_with_shell for MinGW host.
>>>
>>>
>>> Index: fixincludes/fixincl.c
>>> ===================================================================
>>> --- fixincludes/fixincl.c (revision 240386)
>>> +++ fixincludes/fixincl.c (working copy)
>>> @@ -170,7 +170,111 @@
>>> exit (EXIT_SUCCESS);
>>> }
>>>
>>> +#ifdef __MINGW32__
>>>
>>> +/* Count number of \c needle character instances in string */
>>> +static int
>>> +count_chars ( char* str, char needle )
>>
>> Formatting it. This should be:
>>
>> count_chars (char* str, char needle)
>>
>> Note the lack of horizontal whitespace after the open paren or before
>> the close paren. Similarly for system_with_shell declaration below.
>>
>> Wouldn't this be better named "count_occurrences_of_char" or
>> "count_instances_of_char"?
>>
>>
>>
>>
>>> +
>>> + /* Allocate enough memory to fit newly created command string */
>>> + cmd_size = strlen (env_shell) + (sizeof (z_shell_start_args) - 1)
>>> + + strlen (s) + newline_cnt * (sizeof(z_shell_newline) - 1
>>> - 1)
>>> + + (sizeof (z_shell_end_args) - 1) + 1;
>>
>> Why use sizeof (foo) - 1 rather than just strlen (foo) here? Note that
>> GCC can compute the string length as a compile time constant, so you're
>> not gaining any performance by using sizeof here and strlen seems clearer.
>>
>>
>>
>>> +
>>> + long_cmd = XNEWVEC (char, cmd_size);
>>> +
>>> + /* Start with ${SHELL} -c " */
>>> + strcpy (long_cmd, env_shell);
>>> + strcat (long_cmd, z_shell_start_args);
>>> +
>>> + /* End pointer for appending pieces of command while replacing
>>> newlines */
>>> + cmd_endp = long_cmd + strlen(long_cmd);
>>
>> Formatting nit. Space between function name and its argument list.
>>
>>
>>> + nl_scan = s;
>>> + while (nl_scan != NULL)
>>> + {
>>> + char* next_nl = strchr (nl_scan, '\n');
>>
>> Formatting nit. char *next_nl.
>>
>>
>>> + if (next_nl)
>>> + {
>>> + /* Append part of command between newlines */
>>> + size_t part_len = next_nl - nl_scan;
>>> + memcpy(cmd_endp, nl_scan, part_len);
>>
>> Formatting nit, space between function name and its argument list.
>>
>>> + cmd_endp += part_len;
>>> +
>>> + /* Append newline replacement */
>>> + memcpy(cmd_endp, z_shell_newline, sizeof(z_shell_newline));
>>
>> Smilarly, space between functio nname and its argument list.
>>
>>> + cmd_endp += sizeof(z_shell_newline) - 1;
>>
>> Here too.
>>
>>> +
>>> + free ( (void*) long_cmd);
>>
>> free (long_cmd);
>>
>> Can you fix those various (minor) issue and resubmit. I think it'll be
>> OK for the trunk with those changes.
>>
>> jeff
>>
>>
>
