PAG support only
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 5 lines
David, please stick with Linus-style. That means commas after spaces and never, ever, ever:
if (foo) bar
So you'd rather have:
long sys_setpag(pag_t pag)
{
if (pag > 0)
return vfs_join_pag(pag);
else if (pag == 0)
return vfs_leave_pag();
else if (pag == -1)
return vfs_new_pag();
else
return -EINVAL;
}Than:
long sys_setpag(pag_t pag)
{
if (pag > 0) return vfs_join_pag(pag);
else if (pag == 0) return vfs_leave_pag();
else if (pag == -1) return vfs_new_pag();
else return -EINVAL;
}When the former is _far_ less readable at a glance? And also consumes nearly twice as many screen lines [see CodingStyle: "Thus, as the supply of new-lines on your screen is not a renewable resource (think 25-line terminal screens here), you have more empty lines to put comments on."].
Quoted 1 line
and syscalls should return long, not int.
Fair enough, but in arch/i386/kernel/process.c:
asmlinkage int sys_fork(struct pt_regs regs) asmlinkage int sys_clone(struct pt_regs regs) asmlinkage int sys_vfork(struct pt_regs regs) asmlinkage int sys_execve(struct pt_regs regs) etc...
Should these be fixed too (the i386 arch is referred to quite a lot)?
David
Quoted 3 lines · David Howells
So you'd rather have:
Doesn't matter.
We want all the code looking the same, and that is more important than the arguable benefit of the minor changes which you propose.
Quoted 7 lines
long sys_setpag(pag_t pag)
{
if (pag > 0) return vfs_join_pag(pag);
else if (pag == 0) return vfs_leave_pag();
else if (pag == -1) return vfs_new_pag();
else return -EINVAL;
}If someone else comes along and makes changes to this, you'll end up with a mix of styles.
Quoted 11 lines
Quoted 1 line
and syscalls should return long, not int.
Fair enough, but in
arch/i386/kernel/process.c:asmlinkage int
sys_fork(struct pt_regs regs)asmlinkage intsys_clone(struct pt_regs regs)asmlinkage intsys_vfork(struct pt_regs regs)asmlinkage intsys_execve(struct pt_regs regs)etc...Should these be fixed too (the i386 arch is referred to quite a lot)?
I don't know really. David M-T said "syscalls should return long". I'm not sure why - perhaps it's only those which need 32-bit wrappers (guess).
Quoted 11 lines · David Howells
So you'd rather have:
if (pag > 0) return vfs_join_pag(pag); else if (pag == 0) return vfs_leave_pag(); else if (pag == -1) return vfs_new_pag(); else return -EINVAL;
Quoted 8 lines
Than:
if (pag > 0) return vfs_join_pag(pag); else if (pag == 0) return vfs_leave_pag(); else if (pag == -1) return vfs_new_pag(); else return -EINVAL;When the former is _far_ less readable at a glance?
Keep in mind that `readable' also means that someone used to the kernel's coding style, but not to your particular portion, should naturally be able to follow the code with as little trouble as possible. While I agree that your second example is more clear once one sees what's going on (and it's certainly prettier in some sense), the `shape' is unusual, so it takes some adaptation on the part of the reader to make this jump, and that's something to be avoided if possible.
My personal feeling is that (1) there are exceptional cases where the rules should be broken, and (2) but not that many.
These exceptional cases usually stick out like a sore thumb, and if there's any doubt, it's probably not one.
-Miles
Signature · 1 line
We live, as we dream -- alone....