On Thu, 5 Mar 2015, Alex Dowad wrote:Do others concur with this idea? Personally, I feel the code will be more readable/maintainable if the naming of args/variables/etc reflects what they are actually used for.
I would recommend exactly "arg" since it can be used for multiple purposesThe second argument is *only* ever used for one thing: an argument passed to adiff --git a/kernel/fork.c b/kernel/fork.cLooks fine, but I'm not sure about commenting functional formals. Since
index cf65139..b38a2ae 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1186,10 +1186,12 @@ init_task_pid(struct task_struct *task, enum
pid_type type, struct pid *pid)
* It copies the registers, and all the appropriate
* parts of the process environment (as per the clone
* flags). The actual kick-off is left to the caller.
+ *
+ * When copying a kernel thread, 'stack_start' is the function to run.
*/
static struct task_struct *copy_process(unsigned long clone_flags,
unsigned long stack_start,
- unsigned long stack_size,
+ unsigned long kthread_arg,
int __user *child_tidptr,
struct pid *pid,
int trace)
@@ -1401,7 +1403,7 @@ static struct task_struct *copy_process(unsigned
long clone_flags,
retval = copy_io(clone_flags, p);
if (retval)
goto bad_fork_cleanup_namespaces;
- retval = copy_thread(clone_flags, stack_start, stack_size, p);
+ retval = copy_thread(clone_flags, stack_start, kthread_arg, p);
if (retval)
goto bad_fork_cleanup_io;
@@ -1629,8 +1631,8 @@ struct task_struct *fork_idle(int cpu)
* it and waits for it to finish using the VM if required.
*/
long do_fork(unsigned long clone_flags,
- unsigned long stack_start,
- unsigned long stack_size,
+ unsigned long stack_start, /* or function for kthread to run */
+ unsigned long kthread_arg,
int __user *parent_tidptr,
int __user *child_tidptr)
{
copy_process() and do_fork() can have formals with different meanings,
then why not just rename them "arg1" and "arg2" respectively and then
define in the comment above the function what the possible combinations
are?
kernel thread. That's why I would like to rename it to "kthread_arg". The
previous argument (currently named "stack_start") is indeed used for 2
different things: a new stack pointer for a user thread, or a function to be
executed by a kernel thread. Rather than "arg1", what would you think of
something like "sp_or_fn", or "usp_or_fn"?
and if the formal could ever be used for a third purpose we don't want to
go through another re-naming patch to change it from sp_or_fn or
usp_or_fn.
If that's done, then the comment above the function could define what arg
can represent.