[PATCH 1/3] rt-tests: Support user supplied CFLAGS and LDFLAGS

[PATCH 1/3] rt-tests: Support user supplied CFLAGS and LDFLAGS

[Darren Hart]

Accept user supplied CFLAGS and LDFLAGS, overwriting the
Makefile supplied versions. This can cause the build to
fail if the user does not provide at least what the Makefile
defines, but so be it.

Signed-off-by: Darren Hart <dvhart@linux.intel.com>
CC: Clark Williams <williams@redhat.com>
CC: John Kacur <jkacur@redhat.com>
CC: Denys Dmytriyenko <denis@denix.org>
---
 Makefile |   25 +++++++++++++------------
 1 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/Makefile b/Makefile
index 4038dcc..e1edf6c 100644
--- a/Makefile
+++ b/Makefile
@@ -20,7 +20,8 @@ ifneq ($(filter x86_64 i386 ia64 mips powerpc,$(machinetype)),)
 NUMA 	:= 1
 endif
 
-CFLAGS = -D_GNU_SOURCE -Wall -Wno-nonnull -Isrc/include
+CFLAGS ?= -D_GNU_SOURCE -Wall -Wno-nonnull -Isrc/include
+LDFLAGS ?=
 
 PYLIB  := $(shell python -c 'import distutils.sysconfig;  print distutils.sysconfig.get_python_lib()')
 
@@ -61,41 +62,41 @@ all: $(TARGETS) hwlatdetect
 -include $(sources:.c=.d)
 
 cyclictest: cyclictest.o rt-utils.o
-	$(CC) $(CFLAGS) -o $@ $^ $(LIBS) $(NUMA_LIBS)
+	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LIBS) $(NUMA_LIBS)
 
 signaltest: signaltest.o rt-utils.o
-	$(CC) $(CFLAGS) -o $@ $^ $(LIBS)
+	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LIBS)
 
 pi_stress: pi_stress.o
-	$(CC) $(CFLAGS) -o $@ $^ $(LIBS)
+	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LIBS)
 
 hwlatdetect:  src/hwlatdetect/hwlatdetect.py
 	chmod +x src/hwlatdetect/hwlatdetect.py
 	ln -s src/hwlatdetect/hwlatdetect.py hwlatdetect
 
 rt-migrate-test: rt-migrate-test.o
-	$(CC) $(CFLAGS) -o $@ $^ $(LIBS)
+	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LIBS)
 
 ptsematest: ptsematest.o rt-utils.o rt-get_cpu.o
-	$(CC) $(CFLAGS) -o $@ $^ $(LIBS) $(EXTRA_LIBS)
+	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LIBS) $(EXTRA_LIBS)
 
 sigwaittest: sigwaittest.o rt-utils.o rt-get_cpu.o
-	$(CC) $(CFLAGS) -o $@ $^ $(LIBS) $(EXTRA_LIBS)
+	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LIBS) $(EXTRA_LIBS)
 
 svsematest: svsematest.o rt-utils.o rt-get_cpu.o
-	$(CC) $(CFLAGS) -o $@ $^ $(LIBS) $(EXTRA_LIBS)
+	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LIBS) $(EXTRA_LIBS)
 
 pmqtest: pmqtest.o rt-utils.o rt-get_cpu.o
-	$(CC) $(CFLAGS) -o $@ $^ $(LIBS) $(EXTRA_LIBS)
+	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LIBS) $(EXTRA_LIBS)
 
 sendme: sendme.o rt-utils.o rt-get_cpu.o
-	$(CC) $(CFLAGS) -o $@ $^ $(LIBS) $(EXTRA_LIBS)
+	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LIBS) $(EXTRA_LIBS)
 
 pip_stress: pip_stress.o error.o rt-utils.o
-	$(CC) $(CFLAGS) -o $@ $^ $(LIBS)
+	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LIBS)
 
 hackbench: hackbench.o
-	$(CC) $(CFLAGS) -o $@ $^ $(LIBS)
+	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LIBS)
 
 CLEANUP  = $(TARGETS) *.o .depend *.*~ *.orig *.rej rt-tests.spec *.d
 CLEANUP += $(if $(wildcard .git), ChangeLog)
-- 
1.7.6.5

[PATCH 2/3] rt-tests: Silence unused-but-set warning in rt-migrate-test

[Darren Hart]

The variable "end" is set and used, but gcc appears to lose track of it
across the call to lgprint when it gets incorporated into the va_list.
Silence the warning using the unused attribute.

Signed-off-by: Darren Hart <dvhart@linux.intel.com>
CC: Clark Williams <williams@redhat.com>
CC: John Kacur <jkacur@redhat.com>
---
 src/rt-migrate-test/rt-migrate-test.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/rt-migrate-test/rt-migrate-test.c b/src/rt-migrate-test/rt-migrate-test.c
index 1963641..d2ac400 100644
--- a/src/rt-migrate-test/rt-migrate-test.c
+++ b/src/rt-migrate-test/rt-migrate-test.c
@@ -532,7 +532,7 @@ int main (int argc, char **argv)
 	logdev_switch_set(1);
 
 	for (loop=0; loop < nr_runs; loop++) {
-		unsigned long long end;
+		unsigned long long __attribute__ ((unused)) end;
 
 		now = get_time();
 
-- 
1.7.6.5

[PATCH 3/3] rt-tests: Remove unused status variable

[Darren Hart]

The status variable is not used. Remove it and avoid the warning from gcc.

Signed-off-by: Darren Hart <dvhart@linux.intel.com>
CC: Clark Williams <williams@redhat.com>
CC: John Kacur <jkacur@redhat.com>
---
 src/pi_tests/pi_stress.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/src/pi_tests/pi_stress.c b/src/pi_tests/pi_stress.c
index 0940567..b89dec8 100644
--- a/src/pi_tests/pi_stress.c
+++ b/src/pi_tests/pi_stress.c
@@ -596,10 +596,9 @@ void *reporter(void *arg)
 
 int verify_cpu(int cpu)
 {
-	int status;
 	cpu_set_t mask;
 
-	status = sched_getaffinity(0, sizeof(cpu_set_t), &mask);
+	sched_getaffinity(0, sizeof(cpu_set_t), &mask);
 
 	if (CPU_ISSET(cpu, &mask))
 		return SUCCESS;
-- 
1.7.6.5

Re: [PATCH 1/3] rt-tests: Support user supplied CFLAGS and LDFLAGS

[Remy Bohmer]

Hi,

2012/3/20 Darren Hart <dvhart@linux.intel.com>:
> Accept user supplied CFLAGS and LDFLAGS, overwriting the
> Makefile supplied versions. This can cause the build to
> fail if the user does not provide at least what the Makefile
> defines, but so be it.
> -CFLAGS = -D_GNU_SOURCE -Wall -Wno-nonnull -Isrc/include
> +CFLAGS ?= -D_GNU_SOURCE -Wall -Wno-nonnull -Isrc/include
> +LDFLAGS ?=

Why not append the Makefile local flags to the user supplied flags?
Something like:
CFLAGS := $(CFLAGS) -D_GNU_SOURCE -Wall -Wno-nonnull -Isrc/include

Kind regards,

Remy

Re: [PATCH 1/3] rt-tests: Support user supplied CFLAGS and LDFLAGS

[Darren Hart]

On 03/20/2012 12:31 PM, Remy Bohmer wrote:
> Hi,
> 
> 2012/3/20 Darren Hart <dvhart@linux.intel.com>:
>> Accept user supplied CFLAGS and LDFLAGS, overwriting the
>> Makefile supplied versions. This can cause the build to
>> fail if the user does not provide at least what the Makefile
>> defines, but so be it.
>> -CFLAGS = -D_GNU_SOURCE -Wall -Wno-nonnull -Isrc/include
>> +CFLAGS ?= -D_GNU_SOURCE -Wall -Wno-nonnull -Isrc/include
>> +LDFLAGS ?=
> 
> Why not append the Makefile local flags to the user supplied flags?
> Something like:
> CFLAGS := $(CFLAGS) -D_GNU_SOURCE -Wall -Wno-nonnull -Isrc/include
> 

I considered that. Ultimately, the goal is to provide more control to
users, especially those who may be integrating the package into a larger
build system. In this case, I think control of which warnings you
display and where the includes come from should be configurable, even if
it enables them to blow their feet off.

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCH 1/3] rt-tests: Support user supplied CFLAGS and LDFLAGS

[Remy Bohmer]

Hi,

2012/3/20 Darren Hart <dvhart@linux.intel.com>:
>
>
> On 03/20/2012 12:31 PM, Remy Bohmer wrote:
>> Hi,
>>
>> 2012/3/20 Darren Hart <dvhart@linux.intel.com>:
>>> Accept user supplied CFLAGS and LDFLAGS, overwriting the
>>> Makefile supplied versions. This can cause the build to
>>> fail if the user does not provide at least what the Makefile
>>> defines, but so be it.
>>> -CFLAGS = -D_GNU_SOURCE -Wall -Wno-nonnull -Isrc/include
>>> +CFLAGS ?= -D_GNU_SOURCE -Wall -Wno-nonnull -Isrc/include
>>> +LDFLAGS ?=
>>
>> Why not append the Makefile local flags to the user supplied flags?
>> Something like:
>> CFLAGS := $(CFLAGS) -D_GNU_SOURCE -Wall -Wno-nonnull -Isrc/include
>>
>
> I considered that. Ultimately, the goal is to provide more control to
> users, especially those who may be integrating the package into a larger
> build system. In this case, I think control of which warnings you
> display and where the includes come from should be configurable, even if
> it enables them to blow their feet off.

OK. Clear. Thanks.

Kind regards,

Remy

Re: [PATCH 1/3] rt-tests: Support user supplied CFLAGS and LDFLAGS

[John Kacur]

On Tue, Mar 20, 2012 at 8:05 PM, Darren Hart <dvhart@linux.intel.com> wrote:
> Accept user supplied CFLAGS and LDFLAGS, overwriting the
> Makefile supplied versions. This can cause the build to
> fail if the user does not provide at least what the Makefile
> defines, but so be it.
>
> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
> CC: Clark Williams <williams@redhat.com>
> CC: John Kacur <jkacur@redhat.com>
> CC: Denys Dmytriyenko <denis@denix.org>
> ---

I was just wondering what you need LDFLAGS for? Chatting with Darren
on IRC, it seems like you're using -Wl to pass options via gcc to the
linker, and we don't have loadable libs either. Maybe you could resend
the patch with just the CFLAGS change until we have a real world
reason for LDFLAGS

Thanks

Re: [PATCH 1/3] rt-tests: Support user supplied CFLAGS and LDFLAGS

[Darren Hart]

On 03/20/2012 04:58 PM, John Kacur wrote:
> On Tue, Mar 20, 2012 at 8:05 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>> Accept user supplied CFLAGS and LDFLAGS, overwriting the
>> Makefile supplied versions. This can cause the build to
>> fail if the user does not provide at least what the Makefile
>> defines, but so be it.
>>
>> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
>> CC: Clark Williams <williams@redhat.com>
>> CC: John Kacur <jkacur@redhat.com>
>> CC: Denys Dmytriyenko <denis@denix.org>
>> ---
> 
> I was just wondering what you need LDFLAGS for? Chatting with Darren
> on IRC, it seems like you're using -Wl to pass options via gcc to the
> linker, and we don't have loadable libs either. Maybe you could resend
> the patch with just the CFLAGS change until we have a real world
> reason for LDFLAGS

Denys,

Am I missing a reason why we need LDFLAGS? With the current Makefile. we
could just add anything we want to CFLAGS in a pinch anyway...

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCH 3/3] rt-tests: Remove unused status variable

[John Kacur]

On Tue, Mar 20, 2012 at 8:05 PM, Darren Hart <dvhart@linux.intel.com> wrote:
> The status variable is not used. Remove it and avoid the warning from gcc.
>
> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
> CC: Clark Williams <williams@redhat.com>
> CC: John Kacur <jkacur@redhat.com>
> ---
>  src/pi_tests/pi_stress.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/src/pi_tests/pi_stress.c b/src/pi_tests/pi_stress.c
> index 0940567..b89dec8 100644
> --- a/src/pi_tests/pi_stress.c
> +++ b/src/pi_tests/pi_stress.c
> @@ -596,10 +596,9 @@ void *reporter(void *arg)
>
>  int verify_cpu(int cpu)
>  {
> -       int status;
>        cpu_set_t mask;
>
> -       status = sched_getaffinity(0, sizeof(cpu_set_t), &mask);
> +       sched_getaffinity(0, sizeof(cpu_set_t), &mask);
>
>        if (CPU_ISSET(cpu, &mask))
>                return SUCCESS;
> --

Don't you think it would be smarter to test the return status of
sched_getaffinity, than to shut-up the warning by removing the status
variable? I say we leave the variable in to remind us that we're not
finished.

Thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] rt-tests: Silence unused-but-set warning in rt-migrate-test

[Steven Rostedt]

On Tue, 2012-03-20 at 12:05 -0700, Darren Hart wrote:

> diff --git a/src/rt-migrate-test/rt-migrate-test.c b/src/rt-migrate-test/rt-migrate-test.c
> index 1963641..d2ac400 100644
> --- a/src/rt-migrate-test/rt-migrate-test.c
> +++ b/src/rt-migrate-test/rt-migrate-test.c
> @@ -532,7 +532,7 @@ int main (int argc, char **argv)
>  	logdev_switch_set(1);
>  
>  	for (loop=0; loop < nr_runs; loop++) {
> -		unsigned long long end;
> +		unsigned long long __attribute__ ((unused)) end;

I don't like this. It seems to be a gcc bug. What version of gcc is
this?

-- Steve


>  
>  		now = get_time();
>  

Re: [PATCH 2/3] rt-tests: Silence unused-but-set warning in rt-migrate-test

[Darren Hart]

On 03/21/2012 07:05 AM, Steven Rostedt wrote:
> On Tue, 2012-03-20 at 12:05 -0700, Darren Hart wrote:
> 
>> diff --git a/src/rt-migrate-test/rt-migrate-test.c b/src/rt-migrate-test/rt-migrate-test.c
>> index 1963641..d2ac400 100644
>> --- a/src/rt-migrate-test/rt-migrate-test.c
>> +++ b/src/rt-migrate-test/rt-migrate-test.c
>> @@ -532,7 +532,7 @@ int main (int argc, char **argv)
>>  	logdev_switch_set(1);
>>  
>>  	for (loop=0; loop < nr_runs; loop++) {
>> -		unsigned long long end;
>> +		unsigned long long __attribute__ ((unused)) end;
> 
> I don't like this. It seems to be a gcc bug. What version of gcc is
> this?
> 

gcc (GCC) 4.6.1 20110908 (Red Hat 4.6.1-9)

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCH 3/3] rt-tests: Remove unused status variable

[Darren Hart]

On 03/21/2012 06:35 AM, John Kacur wrote:
> On Tue, Mar 20, 2012 at 8:05 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>> The status variable is not used. Remove it and avoid the warning from gcc.
>>
>> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
>> CC: Clark Williams <williams@redhat.com>
>> CC: John Kacur <jkacur@redhat.com>
>> ---
>>  src/pi_tests/pi_stress.c |    3 +--
>>  1 files changed, 1 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/pi_tests/pi_stress.c b/src/pi_tests/pi_stress.c
>> index 0940567..b89dec8 100644
>> --- a/src/pi_tests/pi_stress.c
>> +++ b/src/pi_tests/pi_stress.c
>> @@ -596,10 +596,9 @@ void *reporter(void *arg)
>>
>>  int verify_cpu(int cpu)
>>  {
>> -       int status;
>>        cpu_set_t mask;
>>
>> -       status = sched_getaffinity(0, sizeof(cpu_set_t), &mask);
>> +       sched_getaffinity(0, sizeof(cpu_set_t), &mask);
>>
>>        if (CPU_ISSET(cpu, &mask))
>>                return SUCCESS;
>> --
> 
> Don't you think it would be smarter to test the return status of
> sched_getaffinity, than to shut-up the warning by removing the status
> variable? I say we leave the variable in to remind us that we're not
> finished.
> 

My view was that rather than test status, we are testing the cpu mask
explicitly. That should pick up any error.

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCH 2/3] rt-tests: Silence unused-but-set warning in rt-migrate-test

[John Kacur]

On Wed, Mar 21, 2012 at 3:44 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>
>
> On 03/21/2012 07:05 AM, Steven Rostedt wrote:
>> On Tue, 2012-03-20 at 12:05 -0700, Darren Hart wrote:
>>
>>> diff --git a/src/rt-migrate-test/rt-migrate-test.c b/src/rt-migrate-test/rt-migrate-test.c
>>> index 1963641..d2ac400 100644
>>> --- a/src/rt-migrate-test/rt-migrate-test.c
>>> +++ b/src/rt-migrate-test/rt-migrate-test.c
>>> @@ -532,7 +532,7 @@ int main (int argc, char **argv)
>>>      logdev_switch_set(1);
>>>
>>>      for (loop=0; loop < nr_runs; loop++) {
>>> -            unsigned long long end;
>>> +            unsigned long long __attribute__ ((unused)) end;
>>
>> I don't like this. It seems to be a gcc bug. What version of gcc is
>> this?
>>
>
> gcc (GCC) 4.6.1 20110908 (Red Hat 4.6.1-9)
>

Yup, shows up on my F16 machine too, with a slightly different gcc version.
gcc --version
gcc (GCC) 4.6.2 20111027 (Red Hat 4.6.2-1)
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] rt-tests: Silence unused-but-set warning in rt-migrate-test

[Steven Rostedt]

On Wed, 2012-03-21 at 15:56 +0100, John Kacur wrote:
> >
> 
> Yup, shows up on my F16 machine too, with a slightly different gcc version.
> gcc --version
> gcc (GCC) 4.6.2 20111027 (Red Hat 4.6.2-1)

Egad! This is an old version. It still uses *gasp* logdev!

I have a new version that writes into ftrace, and doesn't use #ifdef,
but instead just looks to see if it is added to the kernel or not.

So no, this patch isn't the fix. The real fix is to upgrade to the new
version of the code.

-- Steve

Re: [PATCH 2/3] rt-tests: Silence unused-but-set warning in rt-migrate-test

[Darren Hart]

On 03/21/2012 09:18 AM, Steven Rostedt wrote:
> On Wed, 2012-03-21 at 15:56 +0100, John Kacur wrote:
>>>
>>
>> Yup, shows up on my F16 machine too, with a slightly different gcc version.
>> gcc --version
>> gcc (GCC) 4.6.2 20111027 (Red Hat 4.6.2-1)
> 
> Egad! This is an old version. It still uses *gasp* logdev!
> 
> I have a new version that writes into ftrace, and doesn't use #ifdef,
> but instead just looks to see if it is added to the kernel or not.
> 
> So no, this patch isn't the fix. The real fix is to upgrade to the new
> version of the code.
> 

Of the compiler you mean?

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCH 2/3] rt-tests: Silence unused-but-set warning in rt-migrate-test

[John Kacur]

On Wed, Mar 21, 2012 at 5:36 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>
>
> On 03/21/2012 09:18 AM, Steven Rostedt wrote:
>> On Wed, 2012-03-21 at 15:56 +0100, John Kacur wrote:
>>>>
>>>
>>> Yup, shows up on my F16 machine too, with a slightly different gcc version.
>>> gcc --version
>>> gcc (GCC) 4.6.2 20111027 (Red Hat 4.6.2-1)
>>
>> Egad! This is an old version. It still uses *gasp* logdev!
>>
>> I have a new version that writes into ftrace, and doesn't use #ifdef,
>> but instead just looks to see if it is added to the kernel or not.
>>
>> So no, this patch isn't the fix. The real fix is to upgrade to the new
>> version of the code.
>>
>
> Of the compiler you mean?
>

:) Yup, that's what he meant. Of course this is not a realistic option
for many of us. Can you live with the stupid warning for now instead
of cluttering the code with the annotations?

Thanks
John

Re: [PATCH 2/3] rt-tests: Silence unused-but-set warning in rt-migrate-test

[Steven Rostedt]

On Wed, 2012-03-21 at 09:36 -0700, Darren Hart wrote:

> > So no, this patch isn't the fix. The real fix is to upgrade to the new
> > version of the code.
> > 
> 
> Of the compiler you mean?

No the rt-migrate-test. It doesn't have the logdev #ifdef making the
"end" variable unused.

-- Steve

Re: [PATCH 2/3] rt-tests: Silence unused-but-set warning in rt-migrate-test

[Darren Hart]

On 03/21/2012 09:41 AM, John Kacur wrote:
> On Wed, Mar 21, 2012 at 5:36 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>>
>>
>> On 03/21/2012 09:18 AM, Steven Rostedt wrote:
>>> On Wed, 2012-03-21 at 15:56 +0100, John Kacur wrote:
>>>>>
>>>>
>>>> Yup, shows up on my F16 machine too, with a slightly different gcc version.
>>>> gcc --version
>>>> gcc (GCC) 4.6.2 20111027 (Red Hat 4.6.2-1)
>>>
>>> Egad! This is an old version. It still uses *gasp* logdev!
>>>
>>> I have a new version that writes into ftrace, and doesn't use #ifdef,
>>> but instead just looks to see if it is added to the kernel or not.
>>>
>>> So no, this patch isn't the fix. The real fix is to upgrade to the new
>>> version of the code.
>>>
>>
>> Of the compiler you mean?
>>
> 
> :) Yup, that's what he meant. Of course this is not a realistic option
> for many of us. Can you live with the stupid warning for now instead
> of cluttering the code with the annotations?

Of course, provided this is indeed a gcc bug. I understand we should use
attributes like this very sparingly and I relunctantly sent the patch
thinking it was reasonable for gcc to miss the usage as it gets rolled
into va_list.

Is this resolved in a later version of gcc? Rostedt you apparently use a
newer compiler, can you try a build?

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCH 2/3] rt-tests: Silence unused-but-set warning in rt-migrate-test

[Steven Rostedt]

On Wed, 2012-03-21 at 17:41 +0100, John Kacur wrote:

> > Of the compiler you mean?
> >
> 
> :) Yup, that's what he meant.

Um, no it wasn't.

-- Steve


>  Of course this is not a realistic option
> for many of us. Can you live with the stupid warning for now instead
> of cluttering the code with the annotations?
> 

Re: [PATCH 2/3] rt-tests: Silence unused-but-set warning in rt-migrate-test

[Steven Rostedt]

On Wed, 2012-03-21 at 12:18 -0400, Steven Rostedt wrote:
> On Wed, 2012-03-21 at 15:56 +0100, John Kacur wrote:
> > >
> > 
> > Yup, shows up on my F16 machine too, with a slightly different gcc version.
> > gcc --version
> > gcc (GCC) 4.6.2 20111027 (Red Hat 4.6.2-1)
> 
> Egad! This is an old version. It still uses *gasp* logdev!

Oops, sorry for the confusion.

I started to reply about the compiler (working) and then I saw that
logdev was being used by rt-migrate-test. I was so shocked by the
rt-migrate-test using logdev, I didn't start a new email and just
replied where I left off.

I didn't even notice that I left the version of gcc in my email :-p

-- Steve

Re: [PATCH 2/3] rt-tests: Silence unused-but-set warning in rt-migrate-test

[John Kacur]

On Wed, Mar 21, 2012 at 6:11 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Wed, 2012-03-21 at 12:18 -0400, Steven Rostedt wrote:
>> On Wed, 2012-03-21 at 15:56 +0100, John Kacur wrote:
>> > >
>> >
>> > Yup, shows up on my F16 machine too, with a slightly different gcc version.
>> > gcc --version
>> > gcc (GCC) 4.6.2 20111027 (Red Hat 4.6.2-1)
>>
>> Egad! This is an old version. It still uses *gasp* logdev!
>
> Oops, sorry for the confusion.
>
> I started to reply about the compiler (working) and then I saw that
> logdev was being used by rt-migrate-test. I was so shocked by the
> rt-migrate-test using logdev, I didn't start a new email and just
> replied where I left off.
>
> I didn't even notice that I left the version of gcc in my email :-p
>

Yeah, after I already hit the sent button, I thought, wait a minute,
"logdev", "ftrace", he's talking about the kernel. But once you hit
return, it's out there forever. Sigh.
My question to Darren is still valid though, assuming it is really
gcc, and he already answered that.

Thanks
John

Re: [PATCH 1/3] rt-tests: Support user supplied CFLAGS and LDFLAGS

[Denys Dmytriyenko]

On Tue, Mar 20, 2012 at 05:10:48PM -0700, Darren Hart wrote:
> 
> 
> On 03/20/2012 04:58 PM, John Kacur wrote:
> > On Tue, Mar 20, 2012 at 8:05 PM, Darren Hart <dvhart@linux.intel.com> wrote:
> >> Accept user supplied CFLAGS and LDFLAGS, overwriting the
> >> Makefile supplied versions. This can cause the build to
> >> fail if the user does not provide at least what the Makefile
> >> defines, but so be it.
> >>
> >> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
> >> CC: Clark Williams <williams@redhat.com>
> >> CC: John Kacur <jkacur@redhat.com>
> >> CC: Denys Dmytriyenko <denis@denix.org>
> >> ---
> > 
> > I was just wondering what you need LDFLAGS for? Chatting with Darren
> > on IRC, it seems like you're using -Wl to pass options via gcc to the
> > linker, and we don't have loadable libs either. Maybe you could resend
> > the patch with just the CFLAGS change until we have a real world
> > reason for LDFLAGS
> 
> Denys,
> 
> Am I missing a reason why we need LDFLAGS? With the current Makefile. we
> could just add anything we want to CFLAGS in a pinch anyway...

Darren,

>From OE-Core config files:

LINKER_HASH_STYLE ??= "gnu"
TARGET_LINK_HASH_STYLE ?= "${@['-Wl,--hash-style=gnu',''][d.getVar('LINKER_HASH_STYLE', True) != 'gnu']}"
export TARGET_LDFLAGS = "-Wl,-O1 ${TARGET_LINK_HASH_STYLE}"
ASNEEDED = "-Wl,--as-needed"
TARGET_LDFLAGS += "${ASNEEDED}"
export LDFLAGS = "${TARGET_LDFLAGS}"


So, those are still linker flags (altough passed through -Wl to gcc), hence 
they belong to LDFLAGS, not CFLAGS. Arguably, you only need to pass CFLAGS 
during compile stage and LDFLAGS during link stage. On the other hand, as a 
workaround, I was passing them to TARGET_CC_ARCH, which gets embedded into CC 
and won't distinguish between compile/link stages...

So rt-tests just gets away not using LDFLAGS and re-using CFLAGS for the link 
stage. :) But the current Makefile as it is now won't honor CFLAGS being set 
from outside, unless you pass them explicitly on the command line to make, or 
call make with -e flag.

-- 
Denys

Re: [PATCH 1/3] rt-tests: Support user supplied CFLAGS and LDFLAGS

[Darren Hart]

On 03/21/2012 12:10 PM, Denys Dmytriyenko wrote:
> On Tue, Mar 20, 2012 at 05:10:48PM -0700, Darren Hart wrote:
>>
>>
>> On 03/20/2012 04:58 PM, John Kacur wrote:
>>> On Tue, Mar 20, 2012 at 8:05 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>>>> Accept user supplied CFLAGS and LDFLAGS, overwriting the
>>>> Makefile supplied versions. This can cause the build to
>>>> fail if the user does not provide at least what the Makefile
>>>> defines, but so be it.
>>>>
>>>> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
>>>> CC: Clark Williams <williams@redhat.com>
>>>> CC: John Kacur <jkacur@redhat.com>
>>>> CC: Denys Dmytriyenko <denis@denix.org>
>>>> ---
>>>
>>> I was just wondering what you need LDFLAGS for? Chatting with Darren
>>> on IRC, it seems like you're using -Wl to pass options via gcc to the
>>> linker, and we don't have loadable libs either. Maybe you could resend
>>> the patch with just the CFLAGS change until we have a real world
>>> reason for LDFLAGS
>>
>> Denys,
>>
>> Am I missing a reason why we need LDFLAGS? With the current Makefile. we
>> could just add anything we want to CFLAGS in a pinch anyway...
> 
> Darren,
> 
> From OE-Core config files:
> 
> LINKER_HASH_STYLE ??= "gnu"
> TARGET_LINK_HASH_STYLE ?= "${@['-Wl,--hash-style=gnu',''][d.getVar('LINKER_HASH_STYLE', True) != 'gnu']}"
> export TARGET_LDFLAGS = "-Wl,-O1 ${TARGET_LINK_HASH_STYLE}"
> ASNEEDED = "-Wl,--as-needed"
> TARGET_LDFLAGS += "${ASNEEDED}"
> export LDFLAGS = "${TARGET_LDFLAGS}"
> 
> 
> So, those are still linker flags (altough passed through -Wl to gcc), hence 
> they belong to LDFLAGS, not CFLAGS. Arguably, you only need to pass CFLAGS 
> during compile stage and LDFLAGS during link stage. On the other hand, as a 
> workaround, I was passing them to TARGET_CC_ARCH, which gets embedded into CC 
> and won't distinguish between compile/link stages...
> 
> So rt-tests just gets away not using LDFLAGS and re-using CFLAGS for the link 
> stage. :) But the current Makefile as it is now won't honor CFLAGS being set 
> from outside, unless you pass them explicitly on the command line to make, or 
> call make with -e flag.

Right, at the very least I'll resubmit the patch allowing override of
CFLAGS. But I wanted to know if you felt there was any need to support
overriding of LDFLAGS for rt-tests which doesn't build any shared libs.


-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCH 1/3] rt-tests: Support user supplied CFLAGS and LDFLAGS

[John Kacur]

On Wed, Mar 21, 2012 at 8:11 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>
>
> On 03/21/2012 12:10 PM, Denys Dmytriyenko wrote:
>> On Tue, Mar 20, 2012 at 05:10:48PM -0700, Darren Hart wrote:
>>>
>>>
>>> On 03/20/2012 04:58 PM, John Kacur wrote:
>>>> On Tue, Mar 20, 2012 at 8:05 PM, Darren Hart <dvhart@linux.intel.com> wrote:
>>>>> Accept user supplied CFLAGS and LDFLAGS, overwriting the
>>>>> Makefile supplied versions. This can cause the build to
>>>>> fail if the user does not provide at least what the Makefile
>>>>> defines, but so be it.
>>>>>
>>>>> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
>>>>> CC: Clark Williams <williams@redhat.com>
>>>>> CC: John Kacur <jkacur@redhat.com>
>>>>> CC: Denys Dmytriyenko <denis@denix.org>
>>>>> ---
>>>>
>>>> I was just wondering what you need LDFLAGS for? Chatting with Darren
>>>> on IRC, it seems like you're using -Wl to pass options via gcc to the
>>>> linker, and we don't have loadable libs either. Maybe you could resend
>>>> the patch with just the CFLAGS change until we have a real world
>>>> reason for LDFLAGS
>>>
>>> Denys,
>>>
>>> Am I missing a reason why we need LDFLAGS? With the current Makefile. we
>>> could just add anything we want to CFLAGS in a pinch anyway...
>>
>> Darren,
>>
>> From OE-Core config files:
>>
>> LINKER_HASH_STYLE ??= "gnu"
>> TARGET_LINK_HASH_STYLE ?= "${@['-Wl,--hash-style=gnu',''][d.getVar('LINKER_HASH_STYLE', True) != 'gnu']}"
>> export TARGET_LDFLAGS = "-Wl,-O1 ${TARGET_LINK_HASH_STYLE}"
>> ASNEEDED = "-Wl,--as-needed"
>> TARGET_LDFLAGS += "${ASNEEDED}"
>> export LDFLAGS = "${TARGET_LDFLAGS}"
>>
>>
>> So, those are still linker flags (altough passed through -Wl to gcc), hence
>> they belong to LDFLAGS, not CFLAGS. Arguably, you only need to pass CFLAGS
>> during compile stage and LDFLAGS during link stage. On the other hand, as a
>> workaround, I was passing them to TARGET_CC_ARCH, which gets embedded into CC
>> and won't distinguish between compile/link stages...
>>
>> So rt-tests just gets away not using LDFLAGS and re-using CFLAGS for the link
>> stage. :) But the current Makefile as it is now won't honor CFLAGS being set
>> from outside, unless you pass them explicitly on the command line to make, or
>> call make with -e flag.
>
> Right, at the very least I'll resubmit the patch allowing override of
> CFLAGS. But I wanted to know if you felt there was any need to support
> overriding of LDFLAGS for rt-tests which doesn't build any shared libs.
>

Okay, never mind, you've convinced me, and your changes don't look
like they will break anything, so I'll give you my approval as is.

Thanks.
John