Some trivial build warning fixes and cleanups for UML.
Jan Kiszka (5):
uml: Remove unused variable from line driver
uml: Drop private round_down definition
uml: Fix warning due to missing task_struct declaration
uml: i386: Avoid redefinition of NR_syscalls
uml: Clean up asm/system.h
arch/um/drivers/line.c | 1 -
arch/um/include/asm/system.h | 3 ---
arch/um/kernel/skas/syscall.c | 4 ++--
arch/um/sys-i386/asm/elf.h | 2 ++
arch/um/sys-x86_64/asm/elf.h | 2 ++
arch/um/sys-x86_64/signal.c | 2 --
6 files changed, 6 insertions(+), 8 deletions(-)
The i386 subarch happens to pull in original NR_syscalls. Maybe we can
make that work for all host arch, but for now just avoid the clash by
using an all-upper-case name.
Signed-off-by: Jan Kiszka <[email protected]>
---
arch/um/kernel/skas/syscall.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/um/kernel/skas/syscall.c b/arch/um/kernel/skas/syscall.c
index 4e3b820..f5173e1 100644
--- a/arch/um/kernel/skas/syscall.c
+++ b/arch/um/kernel/skas/syscall.c
@@ -10,7 +10,7 @@
#include "sysdep/syscalls.h"
extern int syscall_table_size;
-#define NR_syscalls (syscall_table_size / sizeof(void *))
+#define NR_SYSCALLS (syscall_table_size / sizeof(void *))
void handle_syscall(struct uml_pt_regs *r)
{
@@ -30,7 +30,7 @@ void handle_syscall(struct uml_pt_regs *r)
* in case it's a compiler bug.
*/
syscall = UPT_SYSCALL_NR(r);
- if ((syscall >= NR_syscalls) || (syscall < 0))
+ if ((syscall >= NR_SYSCALLS) || (syscall < 0))
result = -ENOSYS;
else result = EXECUTE_SYSCALL(syscall, regs);
--
1.6.0.2
Signed-off-by: Jan Kiszka <[email protected]>
---
arch/um/drivers/line.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/arch/um/drivers/line.c b/arch/um/drivers/line.c
index 7a656bd..7f7338c 100644
--- a/arch/um/drivers/line.c
+++ b/arch/um/drivers/line.c
@@ -19,7 +19,6 @@ static irqreturn_t line_interrupt(int irq, void *data)
{
struct chan *chan = data;
struct line *line = chan->line;
- struct tty_struct *tty;
if (line)
chan_interrupt(&line->chan_list, &line->task, line->tty, irq);
--
1.6.0.2
Already defined in kernel.h. The official version assumes that 'n' is
power of two - which it is in our case.
Signed-off-by: Jan Kiszka <[email protected]>
---
arch/um/sys-x86_64/signal.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/arch/um/sys-x86_64/signal.c b/arch/um/sys-x86_64/signal.c
index 1a899a7..07797d1 100644
--- a/arch/um/sys-x86_64/signal.c
+++ b/arch/um/sys-x86_64/signal.c
@@ -165,8 +165,6 @@ struct rt_sigframe
struct _fpstate fpstate;
};
-#define round_down(m, n) (((m) / (n)) * (n))
-
int setup_signal_stack_si(unsigned long stack_top, int sig,
struct k_sigaction *ka, struct pt_regs * regs,
siginfo_t *info, sigset_t *set)
--
1.6.0.2
We can't pull in linux/sched.h, so just declare the struct.
Signed-off-by: Jan Kiszka <[email protected]>
---
arch/um/sys-i386/asm/elf.h | 2 ++
arch/um/sys-x86_64/asm/elf.h | 2 ++
2 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/arch/um/sys-i386/asm/elf.h b/arch/um/sys-i386/asm/elf.h
index e64cd41..a979a22 100644
--- a/arch/um/sys-i386/asm/elf.h
+++ b/arch/um/sys-i386/asm/elf.h
@@ -75,6 +75,8 @@ typedef struct user_i387_struct elf_fpregset_t;
pr_reg[16] = PT_REGS_SS(regs); \
} while (0);
+struct task_struct;
+
extern int elf_core_copy_fpregs(struct task_struct *t, elf_fpregset_t *fpu);
#define ELF_CORE_COPY_FPREGS(t, fpu) elf_core_copy_fpregs(t, fpu)
diff --git a/arch/um/sys-x86_64/asm/elf.h b/arch/um/sys-x86_64/asm/elf.h
index 49655c8..d760967 100644
--- a/arch/um/sys-x86_64/asm/elf.h
+++ b/arch/um/sys-x86_64/asm/elf.h
@@ -95,6 +95,8 @@ typedef struct user_i387_struct elf_fpregset_t;
(pr_reg)[25] = 0; \
(pr_reg)[26] = 0;
+struct task_struct;
+
extern int elf_core_copy_fpregs(struct task_struct *t, elf_fpregset_t *fpu);
#define ELF_CORE_COPY_FPREGS(t, fpu) elf_core_copy_fpregs(t, fpu)
--
1.6.0.2
Remove duplicates and unused prototypes.
Signed-off-by: Jan Kiszka <[email protected]>
---
arch/um/include/asm/system.h | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/arch/um/include/asm/system.h b/arch/um/include/asm/system.h
index 753346e..93af1cf 100644
--- a/arch/um/include/asm/system.h
+++ b/arch/um/include/asm/system.h
@@ -3,11 +3,8 @@
#include "sysdep/system.h"
-extern void *switch_to(void *prev, void *next, void *last);
-
extern int get_signals(void);
extern int set_signals(int enable);
-extern int get_signals(void);
extern void block_signals(void);
extern void unblock_signals(void);
--
1.6.0.2
On Mon, Apr 19, 2010 at 11:53:04PM +0200, Jan Kiszka wrote:
>Signed-off-by: Jan Kiszka <[email protected]>
Acked-by: WANG Cong <[email protected]>
>---
> arch/um/drivers/line.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
>diff --git a/arch/um/drivers/line.c b/arch/um/drivers/line.c
>index 7a656bd..7f7338c 100644
>--- a/arch/um/drivers/line.c
>+++ b/arch/um/drivers/line.c
>@@ -19,7 +19,6 @@ static irqreturn_t line_interrupt(int irq, void *data)
> {
> struct chan *chan = data;
> struct line *line = chan->line;
>- struct tty_struct *tty;
>
> if (line)
> chan_interrupt(&line->chan_list, &line->task, line->tty, irq);
>--
>1.6.0.2
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at http://www.tux.org/lkml/
On Mon, Apr 19, 2010 at 11:53:05PM +0200, Jan Kiszka wrote:
>Already defined in kernel.h. The official version assumes that 'n' is
>power of two - which it is in our case.
>
>Signed-off-by: Jan Kiszka <[email protected]>
>---
> arch/um/sys-x86_64/signal.c | 2 --
> 1 files changed, 0 insertions(+), 2 deletions(-)
>
>diff --git a/arch/um/sys-x86_64/signal.c b/arch/um/sys-x86_64/signal.c
>index 1a899a7..07797d1 100644
>--- a/arch/um/sys-x86_64/signal.c
>+++ b/arch/um/sys-x86_64/signal.c
>@@ -165,8 +165,6 @@ struct rt_sigframe
> struct _fpstate fpstate;
> };
>
>-#define round_down(m, n) (((m) / (n)) * (n))
>-
> int setup_signal_stack_si(unsigned long stack_top, int sig,
> struct k_sigaction *ka, struct pt_regs * regs,
> siginfo_t *info, sigset_t *set)
Shouldn't this signal.c #include <linux/kernel.h>?
Thanks.
On Mon, Apr 19, 2010 at 11:53:06PM +0200, Jan Kiszka wrote:
>We can't pull in linux/sched.h, so just declare the struct.
>
Did you meet any build error? If yes, please include it.
Thanks.
>Signed-off-by: Jan Kiszka <[email protected]>
>---
> arch/um/sys-i386/asm/elf.h | 2 ++
> arch/um/sys-x86_64/asm/elf.h | 2 ++
> 2 files changed, 4 insertions(+), 0 deletions(-)
>
>diff --git a/arch/um/sys-i386/asm/elf.h b/arch/um/sys-i386/asm/elf.h
>index e64cd41..a979a22 100644
>--- a/arch/um/sys-i386/asm/elf.h
>+++ b/arch/um/sys-i386/asm/elf.h
>@@ -75,6 +75,8 @@ typedef struct user_i387_struct elf_fpregset_t;
> pr_reg[16] = PT_REGS_SS(regs); \
> } while (0);
>
>+struct task_struct;
>+
> extern int elf_core_copy_fpregs(struct task_struct *t, elf_fpregset_t *fpu);
>
> #define ELF_CORE_COPY_FPREGS(t, fpu) elf_core_copy_fpregs(t, fpu)
>diff --git a/arch/um/sys-x86_64/asm/elf.h b/arch/um/sys-x86_64/asm/elf.h
>index 49655c8..d760967 100644
>--- a/arch/um/sys-x86_64/asm/elf.h
>+++ b/arch/um/sys-x86_64/asm/elf.h
>@@ -95,6 +95,8 @@ typedef struct user_i387_struct elf_fpregset_t;
> (pr_reg)[25] = 0; \
> (pr_reg)[26] = 0;
>
>+struct task_struct;
>+
> extern int elf_core_copy_fpregs(struct task_struct *t, elf_fpregset_t *fpu);
>
> #define ELF_CORE_COPY_FPREGS(t, fpu) elf_core_copy_fpregs(t, fpu)
>--
>1.6.0.2
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at http://www.tux.org/lkml/
On Mon, Apr 19, 2010 at 11:53:07PM +0200, Jan Kiszka wrote:
>The i386 subarch happens to pull in original NR_syscalls. Maybe we can
>make that work for all host arch, but for now just avoid the clash by
>using an all-upper-case name.
>
Where?
>Signed-off-by: Jan Kiszka <[email protected]>
>---
> arch/um/kernel/skas/syscall.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/arch/um/kernel/skas/syscall.c b/arch/um/kernel/skas/syscall.c
>index 4e3b820..f5173e1 100644
>--- a/arch/um/kernel/skas/syscall.c
>+++ b/arch/um/kernel/skas/syscall.c
>@@ -10,7 +10,7 @@
> #include "sysdep/syscalls.h"
>
> extern int syscall_table_size;
>-#define NR_syscalls (syscall_table_size / sizeof(void *))
>+#define NR_SYSCALLS (syscall_table_size / sizeof(void *))
>
> void handle_syscall(struct uml_pt_regs *r)
> {
>@@ -30,7 +30,7 @@ void handle_syscall(struct uml_pt_regs *r)
> * in case it's a compiler bug.
> */
> syscall = UPT_SYSCALL_NR(r);
>- if ((syscall >= NR_syscalls) || (syscall < 0))
>+ if ((syscall >= NR_SYSCALLS) || (syscall < 0))
> result = -ENOSYS;
> else result = EXECUTE_SYSCALL(syscall, regs);
>
>--
>1.6.0.2
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at http://www.tux.org/lkml/
On Mon, Apr 19, 2010 at 11:53:08PM +0200, Jan Kiszka wrote:
>Remove duplicates and unused prototypes.
>
>Signed-off-by: Jan Kiszka <[email protected]>
Acked-by: WANG Cong <[email protected]>
>---
> arch/um/include/asm/system.h | 3 ---
> 1 files changed, 0 insertions(+), 3 deletions(-)
>
>diff --git a/arch/um/include/asm/system.h b/arch/um/include/asm/system.h
>index 753346e..93af1cf 100644
>--- a/arch/um/include/asm/system.h
>+++ b/arch/um/include/asm/system.h
>@@ -3,11 +3,8 @@
>
> #include "sysdep/system.h"
>
>-extern void *switch_to(void *prev, void *next, void *last);
>-
> extern int get_signals(void);
> extern int set_signals(int enable);
>-extern int get_signals(void);
> extern void block_signals(void);
> extern void unblock_signals(void);
>
>--
>1.6.0.2
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at http://www.tux.org/lkml/
On Tue, Apr 20, 2010 at 04:33:58PM +0800, Amerigo Wang wrote:
> On Mon, Apr 19, 2010 at 11:53:05PM +0200, Jan Kiszka wrote:
> >Already defined in kernel.h. The official version assumes that 'n' is
> >power of two - which it is in our case.
> >
> >Signed-off-by: Jan Kiszka <[email protected]>
> >---
> > arch/um/sys-x86_64/signal.c | 2 --
> > 1 files changed, 0 insertions(+), 2 deletions(-)
> >
> >diff --git a/arch/um/sys-x86_64/signal.c b/arch/um/sys-x86_64/signal.c
> >index 1a899a7..07797d1 100644
> >--- a/arch/um/sys-x86_64/signal.c
> >+++ b/arch/um/sys-x86_64/signal.c
> >@@ -165,8 +165,6 @@ struct rt_sigframe
> > struct _fpstate fpstate;
> > };
> >
> >-#define round_down(m, n) (((m) / (n)) * (n))
> >-
> > int setup_signal_stack_si(unsigned long stack_top, int sig,
> > struct k_sigaction *ka, struct pt_regs * regs,
> > siginfo_t *info, sigset_t *set)
>
> Shouldn't this signal.c #include <linux/kernel.h>?
I agree - if this is going to depend on kernel.h, it should be
explicitly included.
Jeff
On Mon, 19 Apr 2010, Jan Kiszka wrote:
> Signed-off-by: Jan Kiszka <[email protected]>
> ---
> arch/um/drivers/line.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/arch/um/drivers/line.c b/arch/um/drivers/line.c
> index 7a656bd..7f7338c 100644
> --- a/arch/um/drivers/line.c
> +++ b/arch/um/drivers/line.c
> @@ -19,7 +19,6 @@ static irqreturn_t line_interrupt(int irq, void *data)
> {
> struct chan *chan = data;
> struct line *line = chan->line;
> - struct tty_struct *tty;
>
> if (line)
> chan_interrupt(&line->chan_list, &line->task, line->tty, irq);
Applied, thanks.
--
Jiri Kosina
SUSE Labs, Novell Inc.
On Tue, Apr 20, 2010 at 06:09:49PM +0800, Amerigo Wang wrote:
> On Mon, Apr 19, 2010 at 11:53:06PM +0200, Jan Kiszka wrote:
> >We can't pull in linux/sched.h, so just declare the struct.
> >
>
> Did you meet any build error? If yes, please include it.
What does this patch fix, aside from being a bit cleaner?
If it built before, without having a task_struct declaration, I think
that means that the elf_core_copy_fpregs was never used. The
task_struct * in the declaration would become a private task_struct,
known only to the declaration. If the implementation or callers have
the regular task_struct, it will be a different one, and the
prototypes will conflict due to the different types of the first
parameter.
Jeff
--
Work email - jdike at linux dot intel dot com
On Mon, 19 Apr 2010, Jan Kiszka wrote:
> Remove duplicates and unused prototypes.
>
> Signed-off-by: Jan Kiszka <[email protected]>
Applied, thanks.
--
Jiri Kosina
SUSE Labs, Novell Inc.
On Tue, 20 Apr 2010, Amerigo Wang wrote:
> On Mon, Apr 19, 2010 at 11:53:05PM +0200, Jan Kiszka wrote:
> >Already defined in kernel.h. The official version assumes that 'n' is
> >power of two - which it is in our case.
> >
> >Signed-off-by: Jan Kiszka <[email protected]>
> >---
> > arch/um/sys-x86_64/signal.c | 2 --
> > 1 files changed, 0 insertions(+), 2 deletions(-)
> >
> >diff --git a/arch/um/sys-x86_64/signal.c b/arch/um/sys-x86_64/signal.c
> >index 1a899a7..07797d1 100644
> >--- a/arch/um/sys-x86_64/signal.c
> >+++ b/arch/um/sys-x86_64/signal.c
> >@@ -165,8 +165,6 @@ struct rt_sigframe
> > struct _fpstate fpstate;
> > };
> >
> >-#define round_down(m, n) (((m) / (n)) * (n))
> >-
> > int setup_signal_stack_si(unsigned long stack_top, int sig,
> > struct k_sigaction *ka, struct pt_regs * regs,
> > siginfo_t *info, sigset_t *set)
>
> Shouldn't this signal.c #include <linux/kernel.h>?
Well, it gets included implicitly through uaccess.h -> sched.h ->
kernel.h.
Applied, thanks.
--
Jiri Kosina
SUSE Labs, Novell Inc.
On Tue, 20 Apr 2010, Jeff Dike wrote:
> > >Already defined in kernel.h. The official version assumes that 'n' is
> > >power of two - which it is in our case.
> > >
> > >Signed-off-by: Jan Kiszka <[email protected]>
> > >---
> > > arch/um/sys-x86_64/signal.c | 2 --
> > > 1 files changed, 0 insertions(+), 2 deletions(-)
> > >
> > >diff --git a/arch/um/sys-x86_64/signal.c b/arch/um/sys-x86_64/signal.c
> > >index 1a899a7..07797d1 100644
> > >--- a/arch/um/sys-x86_64/signal.c
> > >+++ b/arch/um/sys-x86_64/signal.c
> > >@@ -165,8 +165,6 @@ struct rt_sigframe
> > > struct _fpstate fpstate;
> > > };
> > >
> > >-#define round_down(m, n) (((m) / (n)) * (n))
> > >-
> > > int setup_signal_stack_si(unsigned long stack_top, int sig,
> > > struct k_sigaction *ka, struct pt_regs * regs,
> > > siginfo_t *info, sigset_t *set)
> >
> > Shouldn't this signal.c #include <linux/kernel.h>?
>
> I agree - if this is going to depend on kernel.h, it should be
> explicitly included.
Yup, I have already added that.
Thanks,
--
Jiri Kosina
SUSE Labs, Novell Inc.
On Tue, Apr 20, 2010 at 04:35:52PM +0200, Jiri Kosina wrote:
> On Tue, 20 Apr 2010, Amerigo Wang wrote:
> > Shouldn't this signal.c #include <linux/kernel.h>?
>
> Well, it gets included implicitly through uaccess.h -> sched.h ->
> kernel.h.
You're depending on the internal details of uaccess.h and sched.h. If
either of them changed, then this would cause unexpected compile
failures here.
Better to explicitly include kernel.h.
Jeff
--
Work email - jdike at linux dot intel dot com
Jeff Dike wrote:
> On Tue, Apr 20, 2010 at 06:09:49PM +0800, Amerigo Wang wrote:
>> On Mon, Apr 19, 2010 at 11:53:06PM +0200, Jan Kiszka wrote:
>>> We can't pull in linux/sched.h, so just declare the struct.
>>>
>> Did you meet any build error? If yes, please include it.
>
> What does this patch fix, aside from being a bit cleaner?
CC arch/um/sys-i386/elfcore.o
In file included from /data/linux-2.6/include/linux/elf.h:8,
from /data/linux-2.6/arch/um/sys-i386/elfcore.c:2:
/data/linux-2.6/arch/um/sys-i386/asm/elf.h:78: warning: ‘struct task_struct’ declared inside parameter list
/data/linux-2.6/arch/um/sys-i386/asm/elf.h:78: warning: its scope is only this definition or declaration, which is probably not what you want
I guess not many people build against i386 hosts anymore, so this
remained widely unnoticed.
>
> If it built before, without having a task_struct declaration, I think
> that means that the elf_core_copy_fpregs was never used. The
> task_struct * in the declaration would become a private task_struct,
> known only to the declaration. If the implementation or callers have
> the regular task_struct, it will be a different one, and the
> prototypes will conflict due to the different types of the first
> parameter.
This is just a forward declaration (that many arch elf header include),
so no such problem exists.
BTW, to answer the other question in this thread: We have a circular
dependency that prevents including sched.h.
I can add all these information to some v2 of this patch if it is
required to get this merged. Please let me know.
Jan
Amerigo Wang wrote:
> On Mon, Apr 19, 2010 at 11:53:07PM +0200, Jan Kiszka wrote:
>> The i386 subarch happens to pull in original NR_syscalls. Maybe we can
>> make that work for all host arch, but for now just avoid the clash by
>> using an all-upper-case name.
>>
>
> Where?
Not sure if this answers your question:
CC arch/um/kernel/skas/syscall.o
/data/linux-2.6/arch/um/kernel/skas/syscall.c:13:1: warning:
"NR_syscalls" redefined
In file included from /data/linux-2.6/arch/x86/include/asm/unistd.h:3,
from
/data/linux-2.6/arch/um/sys-i386/shared/sysdep/syscalls.h:6,
from /data/linux-2.6/arch/um/kernel/skas/syscall.c:10:
/data/linux-2.6/arch/x86/include/asm/unistd_32.h:349:1: warning: this is
the location of the previous definition
Jan
On Tue, 20 Apr 2010, Jeff Dike wrote:
> > > Shouldn't this signal.c #include <linux/kernel.h>?
> >
> > Well, it gets included implicitly through uaccess.h -> sched.h ->
> > kernel.h.
>
> You're depending on the internal details of uaccess.h and sched.h. If
> either of them changed, then this would cause unexpected compile
> failures here.
>
> Better to explicitly include kernel.h.
Yeah, I have that already in my tree.
Thanks,
--
Jiri Kosina
SUSE Labs, Novell Inc.
On Tue, 20 Apr 2010, Jan Kiszka wrote:
> >>> We can't pull in linux/sched.h, so just declare the struct.
> >>>
> >> Did you meet any build error? If yes, please include it.
> >
> > What does this patch fix, aside from being a bit cleaner?
>
> CC arch/um/sys-i386/elfcore.o
> In file included from /data/linux-2.6/include/linux/elf.h:8,
> from /data/linux-2.6/arch/um/sys-i386/elfcore.c:2:
> /data/linux-2.6/arch/um/sys-i386/asm/elf.h:78: warning: ‘struct task_struct’ declared inside parameter list
> /data/linux-2.6/arch/um/sys-i386/asm/elf.h:78: warning: its scope is only this definition or declaration, which is probably not what you want
>
> I guess not many people build against i386 hosts anymore, so this
> remained widely unnoticed.
>
> >
> > If it built before, without having a task_struct declaration, I think
> > that means that the elf_core_copy_fpregs was never used. The
> > task_struct * in the declaration would become a private task_struct,
> > known only to the declaration. If the implementation or callers have
> > the regular task_struct, it will be a different one, and the
> > prototypes will conflict due to the different types of the first
> > parameter.
>
> This is just a forward declaration (that many arch elf header include),
> so no such problem exists.
>
> BTW, to answer the other question in this thread: We have a circular
> dependency that prevents including sched.h.
>
> I can add all these information to some v2 of this patch if it is
> required to get this merged. Please let me know.
I have updated the explanation in the changelog and applied the patch. If
anyone has any objections still, please let me know.
Thanks,
--
Jiri Kosina
SUSE Labs, Novell Inc.
On Tue, Apr 20, 2010 at 07:09:15PM +0200, Jan Kiszka wrote:
>Jeff Dike wrote:
>> On Tue, Apr 20, 2010 at 06:09:49PM +0800, Amerigo Wang wrote:
>>> On Mon, Apr 19, 2010 at 11:53:06PM +0200, Jan Kiszka wrote:
>>>> We can't pull in linux/sched.h, so just declare the struct.
>>>>
>>> Did you meet any build error? If yes, please include it.
>>
>> What does this patch fix, aside from being a bit cleaner?
>
> CC arch/um/sys-i386/elfcore.o
>In file included from /data/linux-2.6/include/linux/elf.h:8,
> from /data/linux-2.6/arch/um/sys-i386/elfcore.c:2:
>/data/linux-2.6/arch/um/sys-i386/asm/elf.h:78: warning: ‘struct task_struct’ declared inside parameter list
>/data/linux-2.6/arch/um/sys-i386/asm/elf.h:78: warning: its scope is only this definition or declaration, which is probably not what you want
>
>I guess not many people build against i386 hosts anymore, so this
>remained widely unnoticed.
>
>>
>> If it built before, without having a task_struct declaration, I think
>> that means that the elf_core_copy_fpregs was never used. The
>> task_struct * in the declaration would become a private task_struct,
>> known only to the declaration. If the implementation or callers have
>> the regular task_struct, it will be a different one, and the
>> prototypes will conflict due to the different types of the first
>> parameter.
>
>This is just a forward declaration (that many arch elf header include),
>so no such problem exists.
>
>BTW, to answer the other question in this thread: We have a circular
>dependency that prevents including sched.h.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This is the right reason to do this. Ok then, thanks.
But it looks like x86_64 needs this too.
BTW, I don't think compile warning fixes are trivial enough to go
to [email protected].
On Tue, Apr 20, 2010 at 07:14:43PM +0200, Jan Kiszka wrote:
>Amerigo Wang wrote:
>> On Mon, Apr 19, 2010 at 11:53:07PM +0200, Jan Kiszka wrote:
>>> The i386 subarch happens to pull in original NR_syscalls. Maybe we can
>>> make that work for all host arch, but for now just avoid the clash by
>>> using an all-upper-case name.
>>>
>>
>> Where?
>
>Not sure if this answers your question:
>
> CC arch/um/kernel/skas/syscall.o
>/data/linux-2.6/arch/um/kernel/skas/syscall.c:13:1: warning:
>"NR_syscalls" redefined
>In file included from /data/linux-2.6/arch/x86/include/asm/unistd.h:3,
> from
>/data/linux-2.6/arch/um/sys-i386/shared/sysdep/syscalls.h:6,
> from /data/linux-2.6/arch/um/kernel/skas/syscall.c:10:
>/data/linux-2.6/arch/x86/include/asm/unistd_32.h:349:1: warning: this is
>the location of the previous definition
>
Ah, sure. I misunderstood your purpose, please do include the warning
messages you are trying to fix in your patch description.
Thanks!
On Wed, 21 Apr 2010, Amerigo Wang wrote:
> > CC arch/um/sys-i386/elfcore.o
> >In file included from /data/linux-2.6/include/linux/elf.h:8,
> > from /data/linux-2.6/arch/um/sys-i386/elfcore.c:2:
> >/data/linux-2.6/arch/um/sys-i386/asm/elf.h:78: warning: ‘struct task_struct’ declared inside parameter list
> >/data/linux-2.6/arch/um/sys-i386/asm/elf.h:78: warning: its scope is only this definition or declaration, which is probably not what you want
> >
> >I guess not many people build against i386 hosts anymore, so this
> >remained widely unnoticed.
> >
> >>
> >> If it built before, without having a task_struct declaration, I think
> >> that means that the elf_core_copy_fpregs was never used. The
> >> task_struct * in the declaration would become a private task_struct,
> >> known only to the declaration. If the implementation or callers have
> >> the regular task_struct, it will be a different one, and the
> >> prototypes will conflict due to the different types of the first
> >> parameter.
> >
> >This is just a forward declaration (that many arch elf header include),
> >so no such problem exists.
> >
> >BTW, to answer the other question in this thread: We have a circular
> >dependency that prevents including sched.h.
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This is the right reason to do this. Ok then, thanks.
>
> But it looks like x86_64 needs this too.
>
>
> BTW, I don't think compile warning fixes are trivial enough to go
> to [email protected].
Why?
--
Jiri Kosina
SUSE Labs, Novell Inc.
On Wed, Apr 21, 2010 at 11:38:53AM +0200, Jiri Kosina wrote:
>On Wed, 21 Apr 2010, Amerigo Wang wrote:
>>
>> BTW, I don't think compile warning fixes are trivial enough to go
>> to [email protected].
>
>Why?
>
Usually compile warnings fixes go into -mm, some coding style fixes
too.
On Wed, 21 Apr 2010, Amerigo Wang wrote:
> >> BTW, I don't think compile warning fixes are trivial enough to go
> >> to [email protected].
> >
> >Why?
> >
> Usually compile warnings fixes go into -mm, some coding style fixes
> too.
Well, I personally don't care that much, submit your patches whereever you
wish.
The point of trivial tree is to take off load from other maintainers
(including Andrew) so that focusing on The Real Things is possible.
See relevant section of Documentation/SubmittingPatches.
But, as I said, I don't care that much. If you want to avoid trivial tree
for some reason, feel free to do so.
--
Jiri Kosina
SUSE Labs, Novell Inc.