[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] tracetool: Forbid argument name 'next'
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] tracetool: Forbid argument name 'next' |
Date: |
Mon, 12 Mar 2012 12:09:15 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:10.0.1) Gecko/20120209 Thunderbird/10.0.1 |
Am 12.03.2012 12:01, schrieb Stefan Hajnoczi:
> On Mon, Mar 12, 2012 at 9:34 AM, Kevin Wolf <address@hidden> wrote:
>> It has happened more than once that patches that look perfectly sane
>> and work with simpletrace broke systemtap because they use 'next' as an
>> argument name for a tracing function. However, 'next' is a keyword for
>> systemtap, so we shouldn't use it.
>>
>> Signed-off-by: Kevin Wolf <address@hidden>
>> ---
>> scripts/tracetool | 4 ++++
>> 1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/scripts/tracetool b/scripts/tracetool
>> index 4c9951d..f892af4 100755
>> --- a/scripts/tracetool
>> +++ b/scripts/tracetool
>> @@ -81,6 +81,10 @@ get_args()
>> args=${1#*\(}
>> args=${args%%\)*}
>> echo "$args"
>> +
>> + if (echo "$args" | grep "[ *]next\($\|[, ]\)" > /dev/null 2>&1); then
>> + echo -e "\n#error 'next' is a bad argument name (clash with
>> systemtap keyword)\n "
>> + fi
>
> Good idea, let's prevent it from being used.
>
> I don't think this is the way to do it because callers will parse
> stdout and we're not guaranteed to be generating C code where #error
> works. Instead, we can echo to stderr and do exit 1.
Yes, we may generate something else additionally. But trace.h is
generated in any case and it will cause a compile error before any non-C
file is used.
I tried the 'exit 1' approach first and it doesn't really work. This
function is called from a subshell, so you don't exit tracetool but just
the one subshell and end up with a broken trace.h that will fail
compilation in the same place, just with a less helpful error message.
Kevin