Hi,
This patch series fixes all the warnings I get when building a kernel for my
laptop. Plus a few whitespace cleanups I found along the way.
Felipe Contreras (5):
Trivial whitespace cleanups
kbuild: fix warning when domainname is not available
kbuild: mkcompile_h: trivial cleanups
acpi: fix trivial warning
acpi: fix trivial warnings caused by previous commmit
arch/x86/include/asm/string_32.h | 1 -
arch/x86/kernel/tsc.c | 2 +-
drivers/acpi/acpica/exfldio.c | 8 ++++----
drivers/acpi/acpica/tbfadt.c | 1 -
include/acpi/actypes.h | 4 ++--
scripts/mkcompile_h | 12 +++++++++---
6 files changed, 16 insertions(+), 12 deletions(-)
Signed-off-by: Felipe Contreras <[email protected]>
---
arch/x86/include/asm/string_32.h | 1 -
arch/x86/kernel/tsc.c | 2 +-
drivers/acpi/acpica/tbfadt.c | 1 -
3 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/string_32.h b/arch/x86/include/asm/string_32.h
index c86f452..ae907e6 100644
--- a/arch/x86/include/asm/string_32.h
+++ b/arch/x86/include/asm/string_32.h
@@ -65,7 +65,6 @@ static __always_inline void *__constant_memcpy(void *to, const void *from,
case 4:
*(int *)to = *(int *)from;
return to;
-
case 3:
*(short *)to = *(short *)from;
*((char *)to + 2) = *((char *)from + 2);
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 71f4368..8a2fc11 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -670,7 +670,7 @@ static int time_cpufreq_notifier(struct notifier_block *nb, unsigned long val,
if ((val == CPUFREQ_PRECHANGE && freq->old < freq->new) ||
(val == CPUFREQ_POSTCHANGE && freq->old > freq->new) ||
(val == CPUFREQ_RESUMECHANGE)) {
- *lpj = cpufreq_scale(loops_per_jiffy_ref, ref_freq, freq->new);
+ *lpj = cpufreq_scale(loops_per_jiffy_ref, ref_freq, freq->new);
tsc_khz = cpufreq_scale(tsc_khz_ref, ref_freq, freq->new);
if (!(freq->flags & CPUFREQ_CONST_LOOPS))
diff --git a/drivers/acpi/acpica/tbfadt.c b/drivers/acpi/acpica/tbfadt.c
index 82b02dc..c016335 100644
--- a/drivers/acpi/acpica/tbfadt.c
+++ b/drivers/acpi/acpica/tbfadt.c
@@ -275,7 +275,6 @@ void acpi_tb_parse_fadt(u32 table_index)
void acpi_tb_create_local_fadt(struct acpi_table_header *table, u32 length)
{
-
/*
* Check if the FADT is larger than the largest table that we expect
* (the ACPI 2.0/3.0 version). If so, truncate the table, and issue
--
1.6.5.rc1
Otherwise we get:
"dnsdomainname: Unknown host"
Signed-off-by: Felipe Contreras <[email protected]>
---
scripts/mkcompile_h | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/scripts/mkcompile_h b/scripts/mkcompile_h
index 6a12dd9..2e09f4a 100755
--- a/scripts/mkcompile_h
+++ b/scripts/mkcompile_h
@@ -66,9 +66,13 @@ UTS_TRUNCATE="sed -e s/\(.\{1,$UTS_LEN\}\).*/\1/"
echo \#define LINUX_COMPILE_HOST \"`hostname | $UTS_TRUNCATE`\"
if [ -x /bin/dnsdomainname ]; then
- echo \#define LINUX_COMPILE_DOMAIN \"`dnsdomainname | $UTS_TRUNCATE`\"
+ domain=`dnsdomainname 2> /dev/null`
elif [ -x /bin/domainname ]; then
- echo \#define LINUX_COMPILE_DOMAIN \"`domainname | $UTS_TRUNCATE`\"
+ domain=`domainname 2> /dev/null`
+ fi
+
+ if $domain; then
+ echo \#define LINUX_COMPILE_DOMAIN \"`echo $domain | $UTS_TRUNCATE`\"
else
echo \#define LINUX_COMPILE_DOMAIN
fi
--
1.6.5.rc1
UTS_TRUNCATTE is simpler this way, and now editors idetify this as a
shell script.
Signed-off-by: Felipe Contreras <[email protected]>
---
scripts/mkcompile_h | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/scripts/mkcompile_h b/scripts/mkcompile_h
index 2e09f4a..7ab9c52 100755
--- a/scripts/mkcompile_h
+++ b/scripts/mkcompile_h
@@ -1,3 +1,5 @@
+#!/bin/sh
+
TARGET=$1
ARCH=$2
SMP=$3
@@ -50,7 +52,7 @@ UTS_VERSION="$UTS_VERSION $CONFIG_FLAGS $TIMESTAMP"
# Truncate to maximum length
UTS_LEN=64
-UTS_TRUNCATE="sed -e s/\(.\{1,$UTS_LEN\}\).*/\1/"
+UTS_TRUNCATE="cut -b -$UTS_LEN"
# Generate a temporary compile.h
--
1.6.5.rc1
drivers/acpi/acpica/tbfadt.c: In function ‘acpi_tb_create_local_fadt’:
arch/x86/include/asm/string_32.h:74: warning: array subscript is above array bounds
Signed-off-by: Felipe Contreras <[email protected]>
---
include/acpi/actypes.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
index 37ba576..e312be7 100644
--- a/include/acpi/actypes.h
+++ b/include/acpi/actypes.h
@@ -438,8 +438,8 @@ typedef unsigned long long acpi_integer;
#define ACPI_SET_BIT(target,bit) ((target) |= (bit))
#define ACPI_CLEAR_BIT(target,bit) ((target) &= ~(bit))
-#define ACPI_MIN(a,b) (((a)<(b))?(a):(b))
-#define ACPI_MAX(a,b) (((a)>(b))?(a):(b))
+#define ACPI_MIN(a,b) min(a, b)
+#define ACPI_MAX(a,b) max(a, b)
/* Size calculation */
--
1.6.5.rc1
drivers/acpi/acpica/exfldio.c: In function ‘acpi_ex_extract_from_field’:
drivers/acpi/acpica/exfldio.c:761: warning: comparison of distinct pointer types lacks a cast
drivers/acpi/acpica/exfldio.c:761: warning: comparison of distinct pointer types lacks a cast
drivers/acpi/acpica/exfldio.c:761: warning: comparison of distinct pointer types lacks a cast
drivers/acpi/acpica/exfldio.c:780: warning: comparison of distinct pointer types lacks a cast
drivers/acpi/acpica/exfldio.c:780: warning: comparison of distinct pointer types lacks a cast
drivers/acpi/acpica/exfldio.c:780: warning: comparison of distinct pointer types lacks a cast
drivers/acpi/acpica/exfldio.c: In function ‘acpi_ex_insert_into_field’:
drivers/acpi/acpica/exfldio.c:880: warning: comparison of distinct pointer types lacks a cast
drivers/acpi/acpica/exfldio.c:880: warning: comparison of distinct pointer types lacks a cast
drivers/acpi/acpica/exfldio.c:880: warning: comparison of distinct pointer types lacks a cast
drivers/acpi/acpica/exfldio.c:933: warning: comparison of distinct pointer types lacks a cast
drivers/acpi/acpica/exfldio.c:933: warning: comparison of distinct pointer types lacks a cast
drivers/acpi/acpica/exfldio.c:933: warning: comparison of distinct pointer types lacks a cast
Signed-off-by: Felipe Contreras <[email protected]>
---
drivers/acpi/acpica/exfldio.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/acpica/exfldio.c b/drivers/acpi/acpica/exfldio.c
index 6687be1..4affc5f 100644
--- a/drivers/acpi/acpica/exfldio.c
+++ b/drivers/acpi/acpica/exfldio.c
@@ -760,7 +760,7 @@ acpi_ex_extract_from_field(union acpi_operand_object *obj_desc,
ACPI_MEMCPY(((char *)buffer) + buffer_offset, &merged_datum,
ACPI_MIN(obj_desc->common_field.access_byte_width,
- buffer_length - buffer_offset));
+ (u8)(buffer_length - buffer_offset)));
buffer_offset += obj_desc->common_field.access_byte_width;
merged_datum =
@@ -779,7 +779,7 @@ acpi_ex_extract_from_field(union acpi_operand_object *obj_desc,
ACPI_MEMCPY(((char *)buffer) + buffer_offset, &merged_datum,
ACPI_MIN(obj_desc->common_field.access_byte_width,
- buffer_length - buffer_offset));
+ (u8)(buffer_length - buffer_offset)));
return_ACPI_STATUS(AE_OK);
}
@@ -879,7 +879,7 @@ acpi_ex_insert_into_field(union acpi_operand_object *obj_desc,
ACPI_MEMCPY(&raw_datum, buffer,
ACPI_MIN(obj_desc->common_field.access_byte_width,
- buffer_length - buffer_offset));
+ (u8)(buffer_length - buffer_offset)));
merged_datum =
raw_datum << obj_desc->common_field.start_field_bit_offset;
@@ -932,7 +932,7 @@ acpi_ex_insert_into_field(union acpi_operand_object *obj_desc,
buffer_offset += obj_desc->common_field.access_byte_width;
ACPI_MEMCPY(&raw_datum, ((char *)buffer) + buffer_offset,
ACPI_MIN(obj_desc->common_field.access_byte_width,
- buffer_length - buffer_offset));
+ (u8)(buffer_length - buffer_offset)));
merged_datum |=
raw_datum << obj_desc->common_field.start_field_bit_offset;
}
--
1.6.5.rc1
On Sun, Sep 13, 2009 at 15:38, Felipe Contreras wrote:
> + if $domain; then
is this really correct ? i think you meant to use:
[ -n "$domain" ]
-mike
On Mon, Sep 14, 2009 at 12:12 AM, Mike Frysinger <[email protected]> wrote:
> On Sun, Sep 13, 2009 at 15:38, Felipe Contreras wrote:
>> + if $domain; then
>
> is this really correct ? i think you meant to use:
> [ -n "$domain" ]
What is the difference?
$domain unset
test -n "" -> false
test -> false
$domain is a valid string
test -n "string" -> true
test "string" -> true
The relevant rules are these:
* An omitted EXPRESSION defaults to false
* STRING equivalent to -n STRING
--
Felipe Contreras
.
On Sun, Sep 13, 2009 at 17:42, Felipe Contreras
<[email protected]> wrote:
> On Mon, Sep 14, 2009 at 12:12 AM, Mike Frysinger <[email protected]> wrote:
>> On Sun, Sep 13, 2009 at 15:38, Felipe Contreras wrote:
>>> + if $domain; then
>>
>> is this really correct ? i think you meant to use:
>> [ -n "$domain" ]
>
> What is the difference?
>
> $domain unset
> test -n "" -> false
> test -> false
>
> $domain is a valid string
> test -n "string" -> true
> test "string" -> true
except that you didnt invoke `test` anywhere. you're executing the
contents of $domain.
-mike
On Mon, Sep 14, 2009 at 12:58 AM, Mike Frysinger <[email protected]> wrote:
> .
> On Sun, Sep 13, 2009 at 17:42, Felipe Contreras
> <[email protected]> wrote:
>> On Mon, Sep 14, 2009 at 12:12 AM, Mike Frysinger <[email protected]> wrote:
>>> On Sun, Sep 13, 2009 at 15:38, Felipe Contreras wrote:
>>>> + if $domain; then
>>>
>>> is this really correct ? i think you meant to use:
>>> [ -n "$domain" ]
>>
>> What is the difference?
>>
>> $domain unset
>> test -n "" -> false
>> test -> false
>>
>> $domain is a valid string
>> test -n "string" -> true
>> test "string" -> true
>
> except that you didnt invoke `test` anywhere. you're executing the
> contents of $domain.
Ahh, I'll update it to:
[ "$domain" ]
--
Felipe Contreras
On Mon, Sep 14, 2009 at 01:04:11AM +0300, Felipe Contreras wrote:
> On Mon, Sep 14, 2009 at 12:58 AM, Mike Frysinger <[email protected]> wrote:
> > .
> > On Sun, Sep 13, 2009 at 17:42, Felipe Contreras
> > <[email protected]> wrote:
> >> On Mon, Sep 14, 2009 at 12:12 AM, Mike Frysinger <[email protected]> wrote:
> >>> On Sun, Sep 13, 2009 at 15:38, Felipe Contreras wrote:
> >>>> + ?if $domain; then
> >>>
> >>> is this really correct ? ?i think you meant to use:
> >>> [ -n "$domain" ]
> >>
> >> What is the difference?
> >>
> >> $domain unset
> >> test -n "" -> false
> >> test -> false
> >>
> >> $domain is a valid string
> >> test -n "string" -> true
> >> test "string" -> true
> >
> > except that you didnt invoke `test` anywhere. ?you're executing the
> > contents of $domain.
>
> Ahh, I'll update it to:
> [ "$domain" ]
Please use [ -n "$domain" ].
Be explicit about what you do.
[Likewise in c we never omit "int" just because we can].
Sam
On Mon, Sep 14, 2009 at 7:46 AM, Sam Ravnborg <[email protected]> wrote:
> On Mon, Sep 14, 2009 at 01:04:11AM +0300, Felipe Contreras wrote:
>> On Mon, Sep 14, 2009 at 12:58 AM, Mike Frysinger <[email protected]> wrote:
>> > .
>> > On Sun, Sep 13, 2009 at 17:42, Felipe Contreras
>> > <[email protected]> wrote:
>> >> On Mon, Sep 14, 2009 at 12:12 AM, Mike Frysinger <[email protected]> wrote:
>> >>> On Sun, Sep 13, 2009 at 15:38, Felipe Contreras wrote:
>> >>>> + if $domain; then
>> >>>
>> >>> is this really correct ? i think you meant to use:
>> >>> [ -n "$domain" ]
>> >>
>> >> What is the difference?
>> >>
>> >> $domain unset
>> >> test -n "" -> false
>> >> test -> false
>> >>
>> >> $domain is a valid string
>> >> test -n "string" -> true
>> >> test "string" -> true
>> >
>> > except that you didnt invoke `test` anywhere. you're executing the
>> > contents of $domain.
>>
>> Ahh, I'll update it to:
>> [ "$domain" ]
>
> Please use [ -n "$domain" ].
> Be explicit about what you do.
>
> [Likewise in c we never omit "int" just because we can].
In fact 'int' is implicit of 'signed int', and 'long' is a shorthand
of 'signed long int' and so on. Also, AFAIK 'if (foo)' is preferred
over 'if (foo == true)' or 'if (foo != NULL)' and sometimes even 'if
(foo >= 0)'.
What's the point of going for the explicit form? Make the code less readable?
--
Felipe Contreras
On Mon, Sep 14, 2009 at 04:25, Felipe Contreras wrote:
> On Mon, Sep 14, 2009 at 7:46 AM, Sam Ravnborg wrote:
>> On Mon, Sep 14, 2009 at 01:04:11AM +0300, Felipe Contreras wrote:
>>> On Mon, Sep 14, 2009 at 12:58 AM, Mike Frysinger wrote:
>>> > On Sun, Sep 13, 2009 at 17:42, Felipe Contreras wrote:
>>> >> On Mon, Sep 14, 2009 at 12:12 AM, Mike Frysinger wrote:
>>> >>> On Sun, Sep 13, 2009 at 15:38, Felipe Contreras wrote:
>>> >>>> + if $domain; then
>>> >>>
>>> >>> is this really correct ? i think you meant to use:
>>> >>> [ -n "$domain" ]
>>> >>
>>> >> What is the difference?
>>> >>
>>> >> $domain unset
>>> >> test -n "" -> false
>>> >> test -> false
>>> >>
>>> >> $domain is a valid string
>>> >> test -n "string" -> true
>>> >> test "string" -> true
>>> >
>>> > except that you didnt invoke `test` anywhere. you're executing the
>>> > contents of $domain.
>>>
>>> Ahh, I'll update it to:
>>> [ "$domain" ]
>>
>> Please use [ -n "$domain" ].
>> Be explicit about what you do.
>>
>> [Likewise in c we never omit "int" just because we can].
>
> In fact 'int' is implicit of 'signed int', and 'long' is a shorthand
> of 'signed long int' and so on. Also, AFAIK 'if (foo)' is preferred
> over 'if (foo == true)' or 'if (foo != NULL)' and sometimes even 'if
> (foo >= 0)'.
>
> What's the point of going for the explicit form? Make the code less readable?
your argument here is the opposite of reality. while some of us are
aware of implicit `test`behavior, not everyone is a shell scripting
master. they look at [ "$foo" ] and dont immediately get the
intention. or perhaps someone typoed and didnt actually want -n
semantics. add the whole *3* characters and be done with it.
-mike
On Sun, 2009-09-13 at 22:38 +0300, Felipe Contreras wrote:
> drivers/acpi/acpica/tbfadt.c: In function ‘acpi_tb_create_local_fadt’:
> arch/x86/include/asm/string_32.h:74: warning: array subscript is above array bounds
>
> Signed-off-by: Felipe Contreras <[email protected]>
Could you run this through checkpatch also, it looks like you have a few
style issues..
Daniel
On Mon, Sep 14, 2009 at 4:55 PM, Daniel Walker <[email protected]> wrote:
> On Sun, 2009-09-13 at 22:38 +0300, Felipe Contreras wrote:
>> drivers/acpi/acpica/tbfadt.c: In function ‘acpi_tb_create_local_fadt’:
>> arch/x86/include/asm/string_32.h:74: warning: array subscript is above array bounds
>>
>> Signed-off-by: Felipe Contreras <[email protected]>
>
> Could you run this through checkpatch also, it looks like you have a few
> style issues..
I did... the style issues are already there, my patch is not
introducing them. Do you want me to send a separate patch to fix the
existing style issues?
--
Felipe Contreras
On Mon, 2009-09-14 at 17:34 +0300, Felipe Contreras wrote:
> On Mon, Sep 14, 2009 at 4:55 PM, Daniel Walker <[email protected]> wrote:
> > On Sun, 2009-09-13 at 22:38 +0300, Felipe Contreras wrote:
> >> drivers/acpi/acpica/tbfadt.c: In function ‘acpi_tb_create_local_fadt’:
> >> arch/x86/include/asm/string_32.h:74: warning: array subscript is above array bounds
> >>
> >> Signed-off-by: Felipe Contreras <[email protected]>
> >
> > Could you run this through checkpatch also, it looks like you have a few
> > style issues..
>
> I did... the style issues are already there, my patch is not
> introducing them. Do you want me to send a separate patch to fix the
> existing style issues?
Yes, If your inclined to clean up the whole file, or just the
surrounding code that would be helpful ..
Daniel
On Mon, Sep 14, 2009 at 4:43 PM, Mike Frysinger <[email protected]> wrote:
> On Mon, Sep 14, 2009 at 04:25, Felipe Contreras wrote:
>> On Mon, Sep 14, 2009 at 7:46 AM, Sam Ravnborg wrote:
>>> On Mon, Sep 14, 2009 at 01:04:11AM +0300, Felipe Contreras wrote:
>>>> On Mon, Sep 14, 2009 at 12:58 AM, Mike Frysinger wrote:
>>>> > On Sun, Sep 13, 2009 at 17:42, Felipe Contreras wrote:
>>>> >> On Mon, Sep 14, 2009 at 12:12 AM, Mike Frysinger wrote:
>>>> >>> On Sun, Sep 13, 2009 at 15:38, Felipe Contreras wrote:
>>>> >>>> + if $domain; then
>>>> >>>
>>>> >>> is this really correct ? i think you meant to use:
>>>> >>> [ -n "$domain" ]
>>>> >>
>>>> >> What is the difference?
>>>> >>
>>>> >> $domain unset
>>>> >> test -n "" -> false
>>>> >> test -> false
>>>> >>
>>>> >> $domain is a valid string
>>>> >> test -n "string" -> true
>>>> >> test "string" -> true
>>>> >
>>>> > except that you didnt invoke `test` anywhere. you're executing the
>>>> > contents of $domain.
>>>>
>>>> Ahh, I'll update it to:
>>>> [ "$domain" ]
>>>
>>> Please use [ -n "$domain" ].
>>> Be explicit about what you do.
>>>
>>> [Likewise in c we never omit "int" just because we can].
>>
>> In fact 'int' is implicit of 'signed int', and 'long' is a shorthand
>> of 'signed long int' and so on. Also, AFAIK 'if (foo)' is preferred
>> over 'if (foo == true)' or 'if (foo != NULL)' and sometimes even 'if
>> (foo >= 0)'.
>>
>> What's the point of going for the explicit form? Make the code less readable?
>
> your argument here is the opposite of reality. while some of us are
> aware of implicit `test`behavior, not everyone is a shell scripting
> master. they look at [ "$foo" ] and dont immediately get the
> intention. or perhaps someone typoed and didnt actually want -n
> semantics. add the whole *3* characters and be done with it.
I consider myself an expert in bash (or at least was some time ago)
and I still need to run 'man bash' in order to see what the hell -n
means. On the other hand, what can [ "$foo" ] be confused with? To me
that can be assumed as: $foo is valid.
In any case, I don't see that idiom being used in the source tree
(which I think is bad), and I see -n being used in some places, so
I'll send a new patch using -n for consistency.
Cheers.
--
Felipe Contreras
On Mon, Sep 14, 2009 at 5:02 PM, Daniel Walker <[email protected]> wrote:
> On Mon, 2009-09-14 at 17:34 +0300, Felipe Contreras wrote:
>> I did... the style issues are already there, my patch is not
>> introducing them. Do you want me to send a separate patch to fix the
>> existing style issues?
>
> Yes, If your inclined to clean up the whole file, or just the
> surrounding code that would be helpful ..
Nobody seems to like the patch you suggested me to do... did I wasted
my time doing it?
http://article.gmane.org/gmane.linux.acpi.devel/42383
--
Felipe Contreras
On Thu, 2009-11-12 at 02:23 +0200, Felipe Contreras wrote:
> On Mon, Sep 14, 2009 at 5:02 PM, Daniel Walker <[email protected]> wrote:
> > On Mon, 2009-09-14 at 17:34 +0300, Felipe Contreras wrote:
> >> I did... the style issues are already there, my patch is not
> >> introducing them. Do you want me to send a separate patch to fix the
> >> existing style issues?
> >
> > Yes, If your inclined to clean up the whole file, or just the
> > surrounding code that would be helpful ..
>
> Nobody seems to like the patch you suggested me to do... did I wasted
> my time doing it?
> http://article.gmane.org/gmane.linux.acpi.devel/42383
Did you get some negative feedback ? I don't see any..
Daniel
On Fri, Nov 13, 2009 at 7:43 PM, Daniel Walker <[email protected]> wrote:
> On Thu, 2009-11-12 at 02:23 +0200, Felipe Contreras wrote:
>> On Mon, Sep 14, 2009 at 5:02 PM, Daniel Walker <[email protected]> wrote:
>> > On Mon, 2009-09-14 at 17:34 +0300, Felipe Contreras wrote:
>> >> I did... the style issues are already there, my patch is not
>> >> introducing them. Do you want me to send a separate patch to fix the
>> >> existing style issues?
>> >
>> > Yes, If your inclined to clean up the whole file, or just the
>> > surrounding code that would be helpful ..
>>
>> Nobody seems to like the patch you suggested me to do... did I wasted
>> my time doing it?
>> http://article.gmane.org/gmane.linux.acpi.devel/42383
>
> Did you get some negative feedback ? I don't see any..
Well:
---
And after today's discussion on kernel summit on this topic, I wouldn't
expect any maintainer to merge it, sorry :)
---
That doesn't seem too positive. Or at least there's no indication that
somebody will pick it up.
--
Felipe Contreras
On Fri, Nov 13, 2009 at 6:28 PM, Felipe Contreras
<[email protected]> wrote:
> On Fri, Nov 13, 2009 at 7:43 PM, Daniel Walker <[email protected]> wrote:
>> On Thu, 2009-11-12 at 02:23 +0200, Felipe Contreras wrote:
>>> On Mon, Sep 14, 2009 at 5:02 PM, Daniel Walker <[email protected]> wrote:
>>> > On Mon, 2009-09-14 at 17:34 +0300, Felipe Contreras wrote:
>>> >> I did... the style issues are already there, my patch is not
>>> >> introducing them. Do you want me to send a separate patch to fix the
>>> >> existing style issues?
>>> >
>>> > Yes, If your inclined to clean up the whole file, or just the
>>> > surrounding code that would be helpful ..
>>>
>>> Nobody seems to like the patch you suggested me to do... did I wasted
>>> my time doing it?
>>> http://article.gmane.org/gmane.linux.acpi.devel/42383
>>
>> Did you get some negative feedback ? I don't see any..
>
> Well:
> ---
> And after today's discussion on kernel summit on this topic, I wouldn't
> expect any maintainer to merge it, sorry :)
> ---
>
> That doesn't seem too positive. Or at least there's no indication that
> somebody will pick it up.
I think it is because it doesn't "aggregate" real value to the kernel
(like fixing a crash, that adds value), and the other guys has more
things to do, and don't have time to waste. But that can not be the
true, this is just my point of view.
On Fri, 2009-11-13 at 22:28 +0200, Felipe Contreras wrote:
> On Fri, Nov 13, 2009 at 7:43 PM, Daniel Walker <[email protected]> wrote:
> > On Thu, 2009-11-12 at 02:23 +0200, Felipe Contreras wrote:
> >> On Mon, Sep 14, 2009 at 5:02 PM, Daniel Walker <[email protected]> wrote:
> >> > On Mon, 2009-09-14 at 17:34 +0300, Felipe Contreras wrote:
> >> >> I did... the style issues are already there, my patch is not
> >> >> introducing them. Do you want me to send a separate patch to fix the
> >> >> existing style issues?
> >> >
> >> > Yes, If your inclined to clean up the whole file, or just the
> >> > surrounding code that would be helpful ..
> >>
> >> Nobody seems to like the patch you suggested me to do... did I wasted
> >> my time doing it?
> >> http://article.gmane.org/gmane.linux.acpi.devel/42383
> >
> > Did you get some negative feedback ? I don't see any..
>
> Well:
> ---
> And after today's discussion on kernel summit on this topic, I wouldn't
> expect any maintainer to merge it, sorry :)
> ---
>
> That doesn't seem too positive. Or at least there's no indication that
> somebody will pick it up.
I'd re-submit with a better description of the patch. You should always
try to describe what your doing as accurately as possible so the
maintainer doesn't have to work very hard to know what your doing.. That
particular patch just has a one liner description that wasn't very
informative .. Either that or re-submit your series without that patch
if you don't have confidence in it.
Daniel
On Sun, Nov 15, 2009 at 7:15 PM, Daniel Walker <[email protected]> wrote:
> On Fri, 2009-11-13 at 22:28 +0200, Felipe Contreras wrote:
>> That doesn't seem too positive. Or at least there's no indication that
>> somebody will pick it up.
>
> I'd re-submit with a better description of the patch. You should always
> try to describe what your doing as accurately as possible so the
> maintainer doesn't have to work very hard to know what your doing.
There's nothing to add. It's a patch to cleanup the coding style, that's it.
> That
> particular patch just has a one liner description that wasn't very
> informative .. Either that or re-submit your series without that patch
> if you don't have confidence in it.
All of my patches have been picked up, except the ones for ACPI. I
haven't received a single comment from them, which would explain the
current state of the code.
If somebody raised the hand and said; I'll merge this, please resend,
I'd do that, otherwise I think it's a waste of time.
--
Felipe Contreras
On Sun, 2009-11-15 at 19:27 +0200, Felipe Contreras wrote:
> On Sun, Nov 15, 2009 at 7:15 PM, Daniel Walker <[email protected]> wrote:
> > On Fri, 2009-11-13 at 22:28 +0200, Felipe Contreras wrote:
> >> That doesn't seem too positive. Or at least there's no indication that
> >> somebody will pick it up.
> >
> > I'd re-submit with a better description of the patch. You should always
> > try to describe what your doing as accurately as possible so the
> > maintainer doesn't have to work very hard to know what your doing.
>
> There's nothing to add. It's a patch to cleanup the coding style, that's it.
How do you know the coding style isn't correct? Did you just visually
inspect it?
> > That
> > particular patch just has a one liner description that wasn't very
> > informative .. Either that or re-submit your series without that patch
> > if you don't have confidence in it.
>
> All of my patches have been picked up, except the ones for ACPI. I
> haven't received a single comment from them, which would explain the
> current state of the code.
>
> If somebody raised the hand and said; I'll merge this, please resend,
> I'd do that, otherwise I think it's a waste of time.
Patches often times aren't accepted in the first submission, could be
for lots of reasons. It's your series tho ..
Daniel
You've made changes to automatically generated files, so all the style
changes
would be overwritten when next time these files are generated -- all the
files go through
Lindent, and almost all your changes were introduced by it. If you
really want to make
style changes to ACPICA files, try to play with Lindent, so it produce
what you want.
If you have anything beside style patches, please submit them separately
Regards,
Alex.
Daniel Walker wrote:
> On Sun, 2009-11-15 at 19:27 +0200, Felipe Contreras wrote:
>
>> On Sun, Nov 15, 2009 at 7:15 PM, Daniel Walker <[email protected]> wrote:
>>
>>> On Fri, 2009-11-13 at 22:28 +0200, Felipe Contreras wrote:
>>>
>>>> That doesn't seem too positive. Or at least there's no indication that
>>>> somebody will pick it up.
>>>>
>>> I'd re-submit with a better description of the patch. You should always
>>> try to describe what your doing as accurately as possible so the
>>> maintainer doesn't have to work very hard to know what your doing.
>>>
>> There's nothing to add. It's a patch to cleanup the coding style, that's it.
>>
>
> How do you know the coding style isn't correct? Did you just visually
> inspect it?
>
>
>>> That
>>> particular patch just has a one liner description that wasn't very
>>> informative .. Either that or re-submit your series without that patch
>>> if you don't have confidence in it.
>>>
>> All of my patches have been picked up, except the ones for ACPI. I
>> haven't received a single comment from them, which would explain the
>> current state of the code.
>>
>> If somebody raised the hand and said; I'll merge this, please resend,
>> I'd do that, otherwise I think it's a waste of time.
>>
>
> Patches often times aren't accepted in the first submission, could be
> for lots of reasons. It's your series tho ..
>
> Daniel
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Sun, 2009-11-15 at 20:54 +0300, Alexey Starikovskiy wrote:
> You've made changes to automatically generated files, so all the style
> changes
> would be overwritten when next time these files are generated -- all the
> files go through
> Lindent, and almost all your changes were introduced by it. If you
> really want to make
> style changes to ACPICA files, try to play with Lindent, so it produce
> what you want.
> If you have anything beside style patches, please submit them separately
>
AFAIK, Lindent can't produce checkpatch clean output. It has to do with
various short comings in indent. You really need a human to check up on
it.
Daniel
On Sun, 2009-11-15 at 21:10 +0300, Alexey Starikovskiy wrote:
> >
> It is not really possible. Lindent is not the last step in the process.
What are the steps on the process?
Daniel
Daniel Walker wrote:
> On Sun, 2009-11-15 at 20:54 +0300, Alexey Starikovskiy wrote:
>
>> You've made changes to automatically generated files, so all the style
>> changes
>> would be overwritten when next time these files are generated -- all the
>> files go through
>> Lindent, and almost all your changes were introduced by it. If you
>> really want to make
>> style changes to ACPICA files, try to play with Lindent, so it produce
>> what you want.
>> If you have anything beside style patches, please submit them separately
>>
>>
>
> AFAIK, Lindent can't produce checkpatch clean output. It has to do with
> various short comings in indent. You really need a human to check up on
> it.
>
> Daniel
>
>
It is not really possible. Lindent is not the last step in the process.
On Mon, 2009-11-16 at 01:27 +0800, Felipe Contreras wrote:
> On Sun, Nov 15, 2009 at 7:15 PM, Daniel Walker <[email protected]> wrote:
> > On Fri, 2009-11-13 at 22:28 +0200, Felipe Contreras wrote:
> >> That doesn't seem too positive. Or at least there's no indication that
> >> somebody will pick it up.
> >
> > I'd re-submit with a better description of the patch. You should always
> > try to describe what your doing as accurately as possible so the
> > maintainer doesn't have to work very hard to know what your doing.
>
> There's nothing to add. It's a patch to cleanup the coding style, that's it.
>
> > That
> > particular patch just has a one liner description that wasn't very
> > informative .. Either that or re-submit your series without that patch
> > if you don't have confidence in it.
>
> All of my patches have been picked up, except the ones for ACPI. I
> haven't received a single comment from them, which would explain the
> current state of the code.
Hi, Felipe
All files under linux-2.6/drivers/acpi/acpica and most files under
linux-2.6/include/acpi are ACPICA code, see http://www.acpica.org for
detail.
The ACPICA's coding style is totally different than linux kernel.
For example, a function named AcpiEvaluateObject in ACPICA, but in linux
kernel it is called acpi_evaluate_object.
We have a program "acpisrc" to convert the ACPICA style code to linux
style and then run lindent on the converted code.
You can download the official ACPICA release from
http://www.acpica.org/downloads/ , "acpisrc" is included in it.
It's great that you are contributing to ACPICA code.
All these linuxized ACPICA codes are generated by acpisrc and lindent,
so we'd better not touch those code directly.
Instead, our process is:
1. Write patches for ACPICA code.
2. Send patches to devel.acpica.org, [email protected], [email protected]
3. I will generate and send the linuxized ACPICA code to Len Brown <[email protected]>
4. Len asks Linus to merge the linuxized ACPICA code.
Thanks,
Lin Ming
>
> If somebody raised the hand and said; I'll merge this, please resend,
> I'd do that, otherwise I think it's a waste of time.
>