[PATCH 5/7] um: Add an internal header shared among the user code

Tiwei Bie posted 7 patches 1 year, 9 months ago
There is a newer version of this series
[PATCH 5/7] um: Add an internal header shared among the user code
Posted by Tiwei Bie 1 year, 9 months ago
Move relevant declarations to this header. This will address
below -Wmissing-prototypes warnings:

arch/um/os-Linux/elf_aux.c:26:13: warning: no previous prototype for ‘scan_elf_aux’ [-Wmissing-prototypes]
arch/um/os-Linux/mem.c:213:13: warning: no previous prototype for ‘check_tmpexec’ [-Wmissing-prototypes]
arch/um/os-Linux/skas/process.c:107:6: warning: no previous prototype for ‘wait_stub_done’ [-Wmissing-prototypes]

Signed-off-by: Tiwei Bie <tiwei.btw@antgroup.com>
---
 arch/um/os-Linux/internal.h    | 20 ++++++++++++++++++++
 arch/um/os-Linux/main.c        |  2 --
 arch/um/os-Linux/skas/mem.c    |  2 --
 arch/um/os-Linux/start_up.c    |  2 --
 arch/um/scripts/Makefile.rules |  3 ++-
 5 files changed, 22 insertions(+), 7 deletions(-)
 create mode 100644 arch/um/os-Linux/internal.h

diff --git a/arch/um/os-Linux/internal.h b/arch/um/os-Linux/internal.h
new file mode 100644
index 000000000000..2a0ea7853658
--- /dev/null
+++ b/arch/um/os-Linux/internal.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __UM_OS_LINUX_INTERNAL_H
+#define __UM_OS_LINUX_INTERNAL_H
+
+/*
+ * elf_aux.c
+ */
+extern void scan_elf_aux(char **envp);
+
+/*
+ * mem.c
+ */
+extern void check_tmpexec(void);
+
+/*
+ * skas/process.c
+ */
+extern void wait_stub_done(int pid);
+
+#endif /* __UM_OS_LINUX_INTERNAL_H */
diff --git a/arch/um/os-Linux/main.c b/arch/um/os-Linux/main.c
index e82164f90288..9880cfcb9b8a 100644
--- a/arch/um/os-Linux/main.c
+++ b/arch/um/os-Linux/main.c
@@ -102,8 +102,6 @@ static void setup_env_path(void)
 	}
 }
 
-extern void scan_elf_aux( char **envp);
-
 int __init main(int argc, char **argv, char **envp)
 {
 	char **new_argv;
diff --git a/arch/um/os-Linux/skas/mem.c b/arch/um/os-Linux/skas/mem.c
index 953fb10f3f93..1b0502fb5c75 100644
--- a/arch/um/os-Linux/skas/mem.c
+++ b/arch/um/os-Linux/skas/mem.c
@@ -20,8 +20,6 @@
 
 extern char batch_syscall_stub[], __syscall_stub_start[];
 
-extern void wait_stub_done(int pid);
-
 static inline unsigned long *check_init_stack(struct mm_id * mm_idp,
 					      unsigned long *stack)
 {
diff --git a/arch/um/os-Linux/start_up.c b/arch/um/os-Linux/start_up.c
index 6b21061c431c..f920c837428e 100644
--- a/arch/um/os-Linux/start_up.c
+++ b/arch/um/os-Linux/start_up.c
@@ -222,8 +222,6 @@ static void __init check_ptrace(void)
 	check_sysemu();
 }
 
-extern void check_tmpexec(void);
-
 static void __init check_coredump_limit(void)
 {
 	struct rlimit lim;
diff --git a/arch/um/scripts/Makefile.rules b/arch/um/scripts/Makefile.rules
index a8b7d9dab0a6..b8ea5f3bb1fb 100644
--- a/arch/um/scripts/Makefile.rules
+++ b/arch/um/scripts/Makefile.rules
@@ -9,7 +9,8 @@ USER_OBJS += $(filter %_user.o,$(obj-y) $(USER_SINGLE_OBJS))
 USER_OBJS := $(foreach file,$(USER_OBJS),$(obj)/$(file))
 
 $(USER_OBJS:.o=.%): \
-	c_flags = -Wp,-MD,$(depfile) $(USER_CFLAGS) -include $(srctree)/include/linux/kern_levels.h -include user.h $(CFLAGS_$(basetarget).o)
+	c_flags = -Wp,-MD,$(depfile) $(USER_CFLAGS) -include $(srctree)/include/linux/kern_levels.h \
+		-include user.h -include $(srctree)/arch/um/os-Linux/internal.h $(CFLAGS_$(basetarget).o)
 
 # These are like USER_OBJS but filter USER_CFLAGS through unprofile instead of
 # using it directly.
-- 
2.34.1

Re: [PATCH 5/7] um: Add an internal header shared among the user code
Posted by Johannes Berg 1 year, 9 months ago
On Tue, 2024-04-23 at 19:24 +0800, Tiwei Bie wrote:
> 
>  $(USER_OBJS:.o=.%): \
> -	c_flags = -Wp,-MD,$(depfile) $(USER_CFLAGS) -include $(srctree)/include/linux/kern_levels.h -include user.h $(CFLAGS_$(basetarget).o)
> +	c_flags = -Wp,-MD,$(depfile) $(USER_CFLAGS) -include $(srctree)/include/linux/kern_levels.h \
> +		-include user.h -include $(srctree)/arch/um/os-Linux/internal.h $(CFLAGS_$(basetarget).o)
> 

Why not just include it explicitly? We do have the warnings?

johannes
Re: [PATCH 5/7] um: Add an internal header shared among the user code
Posted by Tiwei Bie 1 year, 9 months ago
On 4/23/24 7:30 PM, Johannes Berg wrote:
> On Tue, 2024-04-23 at 19:24 +0800, Tiwei Bie wrote:
>>
>>  $(USER_OBJS:.o=.%): \
>> -	c_flags = -Wp,-MD,$(depfile) $(USER_CFLAGS) -include $(srctree)/include/linux/kern_levels.h -include user.h $(CFLAGS_$(basetarget).o)
>> +	c_flags = -Wp,-MD,$(depfile) $(USER_CFLAGS) -include $(srctree)/include/linux/kern_levels.h \
>> +		-include user.h -include $(srctree)/arch/um/os-Linux/internal.h $(CFLAGS_$(basetarget).o)
>>
> 
> Why not just include it explicitly?

I think it might be more convenient if we include it implicitly,
especially since there are two levels of directories under os-Linux/.
But I don't have a strong opinion on this. I'm also willing to
include it explicitly.

> We do have the warnings?

Yeah. Without this patch, I can get below warnings with `make ARCH=um defconfig && make ARCH=um`:

arch/um/os-Linux/skas/process.c:107:6: warning: no previous prototype for ‘wait_stub_done’ [-Wmissing-prototypes]
  107 | void wait_stub_done(int pid)
      |      ^~~~~~~~~~~~~~
arch/um/os-Linux/mem.c:213:13: warning: no previous prototype for ‘check_tmpexec’ [-Wmissing-prototypes]
  213 | void __init check_tmpexec(void)
      |             ^~~~~~~~~~~~~

And below warnings with `make ARCH=um SUBARCH=i386 defconfig && make ARCH=um SUBARCH=i386`:

arch/um/os-Linux/elf_aux.c:26:13: warning: no previous prototype for ‘scan_elf_aux’ [-Wmissing-prototypes]
   26 | __init void scan_elf_aux( char **envp)
      |             ^~~~~~~~~~~~
arch/um/os-Linux/skas/process.c:107:6: warning: no previous prototype for ‘wait_stub_done’ [-Wmissing-prototypes]
  107 | void wait_stub_done(int pid)
      |      ^~~~~~~~~~~~~~
arch/um/os-Linux/mem.c:213:13: warning: no previous prototype for ‘check_tmpexec’ [-Wmissing-prototypes]
  213 | void __init check_tmpexec(void)
      |             ^~~~~~~~~~~~~

The compiler I'm using is: gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0

Regards,
Tiwei
Re: [PATCH 5/7] um: Add an internal header shared among the user code
Posted by Johannes Berg 1 year, 9 months ago
On Tue, 2024-04-23 at 20:09 +0800, Tiwei Bie wrote:
> On 4/23/24 7:30 PM, Johannes Berg wrote:
> > On Tue, 2024-04-23 at 19:24 +0800, Tiwei Bie wrote:
> > > 
> > >  $(USER_OBJS:.o=.%): \
> > > -	c_flags = -Wp,-MD,$(depfile) $(USER_CFLAGS) -include $(srctree)/include/linux/kern_levels.h -include user.h $(CFLAGS_$(basetarget).o)
> > > +	c_flags = -Wp,-MD,$(depfile) $(USER_CFLAGS) -include $(srctree)/include/linux/kern_levels.h \
> > > +		-include user.h -include $(srctree)/arch/um/os-Linux/internal.h $(CFLAGS_$(basetarget).o)
> > > 
> > 
> > Why not just include it explicitly?
> 
> I think it might be more convenient if we include it implicitly,
> especially since there are two levels of directories under os-Linux/.
> But I don't have a strong opinion on this. I'm also willing to
> include it explicitly.

Yeah, ok, dunno.

> > We do have the warnings?
> 
> Yeah. Without this patch, I can get below warnings with `make ARCH=um defconfig && make ARCH=um`:
> 

Sure. I meant, we don't need to hide the include, if we need to add it
to some other file, we'll have the warnings as a reminder. :)

I don't think anyone today would write the code as it is now ...

johannes
Re: [PATCH 5/7] um: Add an internal header shared among the user code
Posted by Tiwei Bie 1 year, 9 months ago
On 4/23/24 8:22 PM, Johannes Berg wrote:
> On Tue, 2024-04-23 at 20:09 +0800, Tiwei Bie wrote:
>> On 4/23/24 7:30 PM, Johannes Berg wrote:
>>> On Tue, 2024-04-23 at 19:24 +0800, Tiwei Bie wrote:
>>>>
>>>>  $(USER_OBJS:.o=.%): \
>>>> -	c_flags = -Wp,-MD,$(depfile) $(USER_CFLAGS) -include $(srctree)/include/linux/kern_levels.h -include user.h $(CFLAGS_$(basetarget).o)
>>>> +	c_flags = -Wp,-MD,$(depfile) $(USER_CFLAGS) -include $(srctree)/include/linux/kern_levels.h \
>>>> +		-include user.h -include $(srctree)/arch/um/os-Linux/internal.h $(CFLAGS_$(basetarget).o)
>>>>
>>>
>>> Why not just include it explicitly?
>>
>> I think it might be more convenient if we include it implicitly,
>> especially since there are two levels of directories under os-Linux/.
>> But I don't have a strong opinion on this. I'm also willing to
>> include it explicitly.
> 
> Yeah, ok, dunno.
> 
>>> We do have the warnings?
>>
>> Yeah. Without this patch, I can get below warnings with `make ARCH=um defconfig && make ARCH=um`:
>>
> 
> Sure. I meant, we don't need to hide the include, if we need to add it
> to some other file, we'll have the warnings as a reminder. :)
> 
> I don't think anyone today would write the code as it is now ...

Makes sense. Will include it explicitly. Thanks!

Regards,
Tiwei