[patch] SELinux: trivial, unify iterator variable naming

[patch] SELinux: trivial, unify iterator variable naming

[Vesa-Matti J Kari]

Hello,

Renamed iterators "idx" and "iter" to "i", to be in harmony
with the old unwritten C convention, and with the rest of the code.

Signed-off-by: Vesa-Matti Kari <vmkari@cc.helsinki.fi>

---

Renamed iterators "idx" and "iter" to "i", to be in harmony
with the old unwritten C convention, and with the rest of the code.

 security/selinux/netnode.c   |   38 +++++++++++++++++++-------------------
 security/selinux/netport.c   |   34 +++++++++++++++++-----------------
 security/selinux/selinuxfs.c |   10 +++++-----
 3 files changed, 41 insertions(+), 41 deletions(-)



--- security-testing-2.6/security/selinux/netnode.c	2008-07-20 18:29:22.000000000 +0300
+++ security-testing-2.6-vmk/security/selinux/netnode.c	2008-07-21 00:06:25.000000000 +0300
@@ -128,21 +128,21 @@
  */
 static struct sel_netnode *sel_netnode_find(const void *addr, u16 family)
 {
-	unsigned int idx;
+	unsigned int i;
 	struct sel_netnode *node;

 	switch (family) {
 	case PF_INET:
-		idx = sel_netnode_hashfn_ipv4(*(__be32 *)addr);
+		i = sel_netnode_hashfn_ipv4(*(__be32 *)addr);
 		break;
 	case PF_INET6:
-		idx = sel_netnode_hashfn_ipv6(addr);
+		i = sel_netnode_hashfn_ipv6(addr);
 		break;
 	default:
 		BUG();
 	}

-	list_for_each_entry_rcu(node, &sel_netnode_hash[idx].list, list)
+	list_for_each_entry_rcu(node, &sel_netnode_hash[i].list, list)
 		if (node->nsec.family == family)
 			switch (family) {
 			case PF_INET:
@@ -169,14 +169,14 @@
  */
 static void sel_netnode_insert(struct sel_netnode *node)
 {
-	unsigned int idx;
+	unsigned int i;

 	switch (node->nsec.family) {
 	case PF_INET:
-		idx = sel_netnode_hashfn_ipv4(node->nsec.addr.ipv4);
+		i = sel_netnode_hashfn_ipv4(node->nsec.addr.ipv4);
 		break;
 	case PF_INET6:
-		idx = sel_netnode_hashfn_ipv6(&node->nsec.addr.ipv6);
+		i = sel_netnode_hashfn_ipv6(&node->nsec.addr.ipv6);
 		break;
 	default:
 		BUG();
@@ -186,16 +186,16 @@

 	/* we need to impose a limit on the growth of the hash table so check
 	 * this bucket to make sure it is within the specified bounds */
-	list_add_rcu(&node->list, &sel_netnode_hash[idx].list);
-	if (sel_netnode_hash[idx].size == SEL_NETNODE_HASH_BKT_LIMIT) {
+	list_add_rcu(&node->list, &sel_netnode_hash[i].list);
+	if (sel_netnode_hash[i].size == SEL_NETNODE_HASH_BKT_LIMIT) {
 		struct sel_netnode *tail;
 		tail = list_entry(
-			rcu_dereference(sel_netnode_hash[idx].list.prev),
+			rcu_dereference(sel_netnode_hash[i].list.prev),
 			struct sel_netnode, list);
 		list_del_rcu(&tail->list);
 		call_rcu(&tail->rcu, sel_netnode_free);
 	} else
-		sel_netnode_hash[idx].size++;
+		sel_netnode_hash[i].size++;
 }

 /**
@@ -298,17 +298,17 @@
  */
 static void sel_netnode_flush(void)
 {
-	unsigned int idx;
+	unsigned int i;
 	struct sel_netnode *node, *node_tmp;

 	spin_lock_bh(&sel_netnode_lock);
-	for (idx = 0; idx < SEL_NETNODE_HASH_SIZE; idx++) {
+	for (i = 0; i < SEL_NETNODE_HASH_SIZE; i++) {
 		list_for_each_entry_safe(node, node_tmp,
-					 &sel_netnode_hash[idx].list, list) {
+					 &sel_netnode_hash[i].list, list) {
 				list_del_rcu(&node->list);
 				call_rcu(&node->rcu, sel_netnode_free);
 		}
-		sel_netnode_hash[idx].size = 0;
+		sel_netnode_hash[i].size = 0;
 	}
 	spin_unlock_bh(&sel_netnode_lock);
 }
@@ -325,15 +325,15 @@

 static __init int sel_netnode_init(void)
 {
-	int iter;
+	int i;
 	int ret;

 	if (!selinux_enabled)
 		return 0;

-	for (iter = 0; iter < SEL_NETNODE_HASH_SIZE; iter++) {
-		INIT_LIST_HEAD(&sel_netnode_hash[iter].list);
-		sel_netnode_hash[iter].size = 0;
+	for (i = 0; i < SEL_NETNODE_HASH_SIZE; i++) {
+		INIT_LIST_HEAD(&sel_netnode_hash[i].list);
+		sel_netnode_hash[i].size = 0;
 	}

 	ret = avc_add_callback(sel_netnode_avc_callback, AVC_CALLBACK_RESET,
--- security-testing-2.6/security/selinux/netport.c	2008-07-20 18:29:22.000000000 +0300
+++ security-testing-2.6-vmk/security/selinux/netport.c	2008-07-21 00:09:46.000000000 +0300
@@ -108,11 +108,11 @@
  */
 static struct sel_netport *sel_netport_find(u8 protocol, u16 pnum)
 {
-	unsigned int idx;
+	unsigned int i;
 	struct sel_netport *port;

-	idx = sel_netport_hashfn(pnum);
-	list_for_each_entry_rcu(port, &sel_netport_hash[idx].list, list)
+	i = sel_netport_hashfn(pnum);
+	list_for_each_entry_rcu(port, &sel_netport_hash[i].list, list)
 		if (port->psec.port == pnum && port->psec.protocol == protocol)
 			return port;

@@ -129,21 +129,21 @@
  */
 static void sel_netport_insert(struct sel_netport *port)
 {
-	unsigned int idx;
+	unsigned int i;

 	/* we need to impose a limit on the growth of the hash table so check
 	 * this bucket to make sure it is within the specified bounds */
-	idx = sel_netport_hashfn(port->psec.port);
-	list_add_rcu(&port->list, &sel_netport_hash[idx].list);
-	if (sel_netport_hash[idx].size == SEL_NETPORT_HASH_BKT_LIMIT) {
+	i = sel_netport_hashfn(port->psec.port);
+	list_add_rcu(&port->list, &sel_netport_hash[i].list);
+	if (sel_netport_hash[i].size == SEL_NETPORT_HASH_BKT_LIMIT) {
 		struct sel_netport *tail;
 		tail = list_entry(
-			rcu_dereference(sel_netport_hash[idx].list.prev),
+			rcu_dereference(sel_netport_hash[i].list.prev),
 			struct sel_netport, list);
 		list_del_rcu(&tail->list);
 		call_rcu(&tail->rcu, sel_netport_free);
 	} else
-		sel_netport_hash[idx].size++;
+		sel_netport_hash[i].size++;
 }

 /**
@@ -232,17 +232,17 @@
  */
 static void sel_netport_flush(void)
 {
-	unsigned int idx;
+	unsigned int i;
 	struct sel_netport *port, *port_tmp;

 	spin_lock_bh(&sel_netport_lock);
-	for (idx = 0; idx < SEL_NETPORT_HASH_SIZE; idx++) {
+	for (i = 0; i < SEL_NETPORT_HASH_SIZE; i++) {
 		list_for_each_entry_safe(port, port_tmp,
-					 &sel_netport_hash[idx].list, list) {
+					 &sel_netport_hash[i].list, list) {
 			list_del_rcu(&port->list);
 			call_rcu(&port->rcu, sel_netport_free);
 		}
-		sel_netport_hash[idx].size = 0;
+		sel_netport_hash[i].size = 0;
 	}
 	spin_unlock_bh(&sel_netport_lock);
 }
@@ -259,15 +259,15 @@

 static __init int sel_netport_init(void)
 {
-	int iter;
+	int i;
 	int ret;

 	if (!selinux_enabled)
 		return 0;

-	for (iter = 0; iter < SEL_NETPORT_HASH_SIZE; iter++) {
-		INIT_LIST_HEAD(&sel_netport_hash[iter].list);
-		sel_netport_hash[iter].size = 0;
+	for (i = 0; i < SEL_NETPORT_HASH_SIZE; i++) {
+		INIT_LIST_HEAD(&sel_netport_hash[i].list);
+		sel_netport_hash[i].size = 0;
 	}

 	ret = avc_add_callback(sel_netport_avc_callback, AVC_CALLBACK_RESET,
--- security-testing-2.6/security/selinux/selinuxfs.c	2008-07-20 18:29:22.000000000 +0300
+++ security-testing-2.6-vmk/security/selinux/selinuxfs.c	2008-07-21 00:11:52.000000000 +0300
@@ -1590,16 +1590,16 @@

 static int sel_make_policycap(void)
 {
-	unsigned int iter;
+	unsigned int i;
 	struct dentry *dentry = NULL;
 	struct inode *inode = NULL;

 	sel_remove_entries(policycap_dir);

-	for (iter = 0; iter <= POLICYDB_CAPABILITY_MAX; iter++) {
-		if (iter < ARRAY_SIZE(policycap_names))
+	for (i = 0; i <= POLICYDB_CAPABILITY_MAX; i++) {
+		if (i < ARRAY_SIZE(policycap_names))
 			dentry = d_alloc_name(policycap_dir,
-					      policycap_names[iter]);
+					      policycap_names[i]);
 		else
 			dentry = d_alloc_name(policycap_dir, "unknown");

@@ -1611,7 +1611,7 @@
 			return -ENOMEM;

 		inode->i_fop = &sel_policycap_ops;
-		inode->i_ino = iter | SEL_POLICYCAP_INO_OFFSET;
+		inode->i_ino = i | SEL_POLICYCAP_INO_OFFSET;
 		d_add(dentry, inode);
 	}


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patch] SELinux: trivial, unify iterator variable naming

[Paul Moore]

On Sunday 20 July 2008 5:46:52 pm Vesa-Matti J Kari wrote:
> Hello,
>
> Renamed iterators "idx" and "iter" to "i", to be in harmony
> with the old unwritten C convention, and with the rest of the code.
>
> Signed-off-by: Vesa-Matti Kari <vmkari@cc.helsinki.fi>

Normally I like janitorial fixes such as whitespace cleanups, spelling 
corrections, etc.  However, these three patches seem a bit silly to me 
simply changing a variable name from "idx" to "i" and not adding any 
real value to the code.  If it were up to me alone, I would say "NO", 
but if everyone else agrees they are worthwhile I won't to stand in the 
way.

-- 
paul moore
linux @ hp

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patch] SELinux: trivial, unify iterator variable naming

[Vesa-Matti J Kari]

Hello,

On Sun, 20 Jul 2008, Paul Moore wrote:

> Normally I like janitorial fixes such as whitespace cleanups, spelling
> corrections, etc.  However, these three patches seem a bit silly to me
> simply changing a variable name from "idx" to "i" and not adding any
> real value to the code.  If it were up to me alone, I would say "NO",
> but if everyone else agrees they are worthwhile I won't to stand in the
> way.

I understand, but my preference is that all code should always look as
similar as possible style-wise. To my eyes, it is quite annoying to see,
e.g. "iter" and "idx" used mixed in the same source file, and it distracts
my code reading.

Best regards,
vmk
--
************************************************************************
               Tietotekniikkaosasto / Helsingin yliopisto
                 IT Department / University of Helsinki
************************************************************************

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patch] SELinux: trivial, unify iterator variable naming

[Paul Moore]

On Monday 21 July 2008 4:08:04 am Vesa-Matti J Kari wrote:
> Hello,
>
> On Sun, 20 Jul 2008, Paul Moore wrote:
> > Normally I like janitorial fixes such as whitespace cleanups,
> > spelling corrections, etc.  However, these three patches seem a bit
> > silly to me simply changing a variable name from "idx" to "i" and
> > not adding any real value to the code.  If it were up to me alone,
> > I would say "NO", but if everyone else agrees they are worthwhile I
> > won't to stand in the way.
>
> I understand, but my preference is that all code should always look
> as similar as possible style-wise. To my eyes, it is quite annoying
> to see, e.g. "iter" and "idx" used mixed in the same source file, and
> it distracts my code reading.

The kernel is a very large piece of code, which is developed and 
maintained by literally thousands of people.  While there are rules and 
guidelines to help maintain consistency there will _always_ be 
inconsistent style in the Linux Kernel.  Further, due to the extremely 
high rate of change in the kernel it is doubtful that you will be able 
to stylistically "fix" the entire kernel and maintain it (patch 
acceptance is another issue entirely).

I know it can be frustrating at times, but I would suggest that adapting 
yourself to read the Linux Kernel sources will be much more rewarding 
(both to yourself and to the community) than trying to adapt the kernel 
sources to you.

-- 
paul moore
linux @ hp

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patch] SELinux: trivial, unify iterator variable naming

[Vesa-Matti J Kari]

Hello,

On Mon, 21 Jul 2008, Paul Moore wrote:

> The kernel is a very large piece of code, which is developed and
> maintained by literally thousands of people.  While there are rules and
> guidelines to help maintain consistency there will _always_ be
> inconsistent style in the Linux Kernel.

Sure. But is that really a valid argument to block the changes that
attempt to bring *at least some* unity to the code?

I mean, what you are saying is analogous to the argument, "the world peace
can never be achieved, therefore we should not bother to settle local
issues either".

I certainly don't intend to wade through the entire kernel. I just want to
make some changes to one particular subsystem that I am interested in
(SELinux), to make the reading experience more enjoyable to me and others,
and, hopefully, to somewhat improve the code.

And as I said in my previous message, Linus' CodingStyle-document exists
and is meant to make the code look more uniform, everywhere.

> Further, due to the extremely high rate of change in the kernel it is
> doubtful that you will be able to stylistically "fix" the entire kernel
> and maintain it (patch acceptance is another issue entirely).

I have never set my goal to fix the entire kernel. That should be obvious
considering the huge size of the source tree.

> I know it can be frustrating at times, but I would suggest that adapting
> yourself to read the Linux Kernel sources will be much more rewarding
> (both to yourself and to the community) than trying to adapt the kernel
> sources to you.

It is true that the sources should not be adapted for specifically for me,
but I am quite convinced that we need a one true coding style that we
should try to adhere to. And while coding style deviations might not
be strong enough reasons to reject patches, the coding standard would
allow other people to fix "nonstandard" coding without any extra debates.

Let me take one extra example of what I mean.

As it stands, if I look at one single source file implementing hash
tables, e.g. ss/hashtab.c, I see the following:

hashtab_insert():

  while (cur && h->keycmp(h, key, cur->key) > 0

the very next function, hashtab_search(), has:

  while (cur != NULL && h->keycmp(h, key, cur->key) > 0)



function hashtab_map() has:

  while (cur != NULL)

the very next next function, hashtab_stat() has:

  while (cur) {


I do not like this kind of inconsistency at all.

While the code may be functionally correct, you should write to the reader
as well as to the compiler. Note: I am not at all saying the SELinux code
is bad quality. On the contrary, based on my very limited kernel
knowledge, I think SELinux kernel side is very good.

I just think it could be improved by using unified coding style.

Best regards,
vmk
--
************************************************************************
               Tietotekniikkaosasto / Helsingin yliopisto
                 IT Department / University of Helsinki
************************************************************************

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patch] SELinux: trivial, unify iterator variable naming

[Paul Moore]

On Monday 21 July 2008 9:08:16 am Vesa-Matti J Kari wrote:
> Hello,
>
> On Mon, 21 Jul 2008, Paul Moore wrote:
> > The kernel is a very large piece of code, which is developed and
> > maintained by literally thousands of people.  While there are rules
> > and guidelines to help maintain consistency there will _always_ be
> > inconsistent style in the Linux Kernel.
>
> Sure. But is that really a valid argument to block the changes that
> attempt to bring *at least some* unity to the code?

I obviously thought so which is why I voted "NO".  Not all change is 
good, even if the intentions are correct.  All change brings risk and 
if there is not an advantage to offset that risk then all you are doing 
is adding risk into the system.  Risk of runtime bugs, risk of 
merging/porting bugs, etc.

I don't have a problem with fixing significant style problems in the 
code; I agreed with Eric's massive whitespace cleanup earlier this year 
and I think your patch to convert to '\0' NULL terminators is a good 
one.  However, I do draw the line at patches that only rename 
variables, especially the conversion of 'idx' to 'i'.

> I mean, what you are saying is analogous to the argument, "the world
> peace can never be achieved, therefore we should not bother to settle
> local issues either".

Please stick to the argument at hand and not some overblown analogue.

> I certainly don't intend to wade through the entire kernel. I just
> want to make some changes to one particular subsystem that I am
> interested in (SELinux), to make the reading experience more
> enjoyable to me and others, and, hopefully, to somewhat improve the
> code.

A few thoughts: others did not object to those variable names when the 
patches were proposed, or at any point prior to this thread, and no one 
besides you and I has commented on this thread.  This is a sign to me 
that the 'idx' variable name is not a significant distraction to the 
community.

Another thought, if you want to help improve the code, especially 
SELinux, I would encourage you to look at the ToDo lists:

 * http://www.selinuxproject.org/page/Kernel_Development#To_Do_List
 * http://www.selinuxproject.org/page/Userspace_Development

... and attempt to work on one of the issues on those lists.  I would be 
more than happy to help if you get stuck and need a hand.

-- 
paul moore
linux @ hp

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patch] SELinux: trivial, unify iterator variable naming

[Vesa-Matti J Kari]

Hello,

On Mon, 21 Jul 2008, Paul Moore wrote:

> On Monday 21 July 2008 9:08:16 am Vesa-Matti J Kari wrote:

> > Sure. But is that really a valid argument to block the changes that
> > attempt to bring *at least some* unity to the code?
>
> I obviously thought so which is why I voted "NO".  Not all change is
> good, even if the intentions are correct.

Agreed.

> All change brings risk and if there is not an advantage to offset that
> risk then all you are doing is adding risk into the system.  Risk of
> runtime bugs,

Well, we do have a class of patches that can be applied without risking
the system that much. The class consists of (at least) the following
cases:

1) Code formatting fixes that affect only whitespace. I guess
   these are known as "whitespace patches", and can be considered
   trivial.

2) Code fixes that transform statements into semantically equivalent
   statements in a such a way that the code generated by the
   compiler is equivalent before and after the patch is applied.

I don't know whether that class has any common name, but it is pretty much
risk free, because you just have two source trees; the original and
the patched, and after patching you compare the generated object
files in both trees. If they are identical, then the patching has been
a safe operation to do.

But, yes, you say:

> risk of merging/porting bugs, etc.

Merging can indeed be problematic even with the class mentioned above.

> I don't have a problem with fixing significant style problems in the
> code; I agreed with Eric's massive whitespace cleanup earlier this year
> and I think your patch to convert to '\0' NULL terminators is a good
> one.  However, I do draw the line at patches that only rename
> variables, especially the conversion of 'idx' to 'i'.

That's all right. It's good to know where the limits are.

> [...]
> Another thought, if you want to help improve the code, especially
> SELinux, I would encourage you to look at the ToDo lists:
>
>  * http://www.selinuxproject.org/page/Kernel_Development#To_Do_List
>  * http://www.selinuxproject.org/page/Userspace_Development
>
> ... and attempt to work on one of the issues on those lists.  I would be
> more than happy to help if you get stuck and need a hand.

Thanks for the links. The problem is I have to learn to walk properly
before I can run. Only the very first kernel problem seemed solvable by
me, and it would probably take lots of time, and I'm not 100% confident
about the solution either.

For sure I hope to be able to contribute more than just trivial patches,
but as I go through the code, I'll pick up things that are easily fixed
without any extra effort.

Those are mostly boring fixes, but they should help too, as it enables the
more experienced developers to focus on the more real, high-level problems
concerning the overall architecture and so on. But yeah, I have to say I
have found the "SELinux by Example" book very useful. The code makes much
more sense when you have a general description of the system.

Best regards,
vmk
-- 
************************************************************************
               Tietotekniikkaosasto / Helsingin yliopisto
                 IT Department / University of Helsinki
************************************************************************

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patch] SELinux: trivial, unify iterator variable naming

[Paul Moore]

On Tuesday 22 July 2008 2:21:11 pm Vesa-Matti J Kari wrote:
> On Mon, 21 Jul 2008, Paul Moore wrote:
> > Another thought, if you want to help improve the code, especially
> > SELinux, I would encourage you to look at the ToDo lists:
> >
> >  * http://www.selinuxproject.org/page/Kernel_Development#To_Do_List
> >  * http://www.selinuxproject.org/page/Userspace_Development
> >
> > ... and attempt to work on one of the issues on those lists.  I
> > would be more than happy to help if you get stuck and need a hand.
>
> Thanks for the links. The problem is I have to learn to walk properly
> before I can run. Only the very first kernel problem seemed solvable
> by me, and it would probably take lots of time, and I'm not 100%
> confident about the solution either.

I understand, it can seem a bit daunting a first but don't hesitate to 
ask questions when you are unsure.  In general, everyone here on the 
SELinux list is pretty nice and helpful (although I can get a little 
ornery from time to time :) ) and I for one would love to see more 
involvement from other developers.

-- 
paul moore
linux @ hp

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patch] SELinux: trivial, unify iterator variable naming

[Russell Coker]

On Monday 21 July 2008 23:38, Paul Moore <paul.moore@hp.com> wrote:
> A few thoughts: others did not object to those variable names when the
> patches were proposed, or at any point prior to this thread, and no one
> besides you and I has commented on this thread.  This is a sign to me
> that the 'idx' variable name is not a significant distraction to the
> community.

One thing that is worth considering is the possibility that idx might be 
better for searching.

Lots of words contain an 'i' character and therefore searching for a variable 
named 'i' in a code base is going to be difficult (automated searching is 
mostly useless, probably the best option is to have your editor highlight 
every instance and visually scan for the ones with are not surrounded by 
brackets, braces, parenthesis, spaces, commas, or whatever else is not 
acceptable in a variable name in the language in question.

Of course if you have a syntax highlighting editor then it might parse enough 
of the language to avoid this (but that isn't always available), and if you 
have a language such as Perl a search for \$i should work reasonably well.

http://etbe.coker.com.au/2008/07/30/variable-names/

But using idx, index, or even names such as "count" which refer to the meaning 
of the program might be more efficient overall.  I've posted some more 
thoughts on this issue at the above URL.

-- 
russell@coker.com.au
http://etbe.coker.com.au/          My Blog

http://www.coker.com.au/sponsorship.html Sponsoring Free Software development


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patch] SELinux: trivial, unify iterator variable naming

[Paul Moore]

On Tuesday 29 July 2008 10:47:59 pm Russell Coker wrote:
> On Monday 21 July 2008 23:38, Paul Moore <paul.moore@hp.com> wrote:
> > A few thoughts: others did not object to those variable names when
> > the patches were proposed, or at any point prior to this thread,
> > and no one besides you and I has commented on this thread.  This is
> > a sign to me that the 'idx' variable name is not a significant
> > distraction to the community.
>
> One thing that is worth considering is the possibility that idx might
> be better for searching.
>
> Lots of words contain an 'i' character and therefore searching for a
> variable named 'i' in a code base is going to be difficult (automated
> searching is mostly useless, probably the best option is to have your
> editor highlight every instance and visually scan for the ones with
> are not surrounded by brackets, braces, parenthesis, spaces, commas,
> or whatever else is not acceptable in a variable name in the language
> in question.
>
> Of course if you have a syntax highlighting editor then it might
> parse enough of the language to avoid this (but that isn't always
> available), and if you have a language such as Perl a search for \$i
> should work reasonably well.
>
> http://etbe.coker.com.au/2008/07/30/variable-names/
>
> But using idx, index, or even names such as "count" which refer to
> the meaning of the program might be more efficient overall.  I've
> posted some more thoughts on this issue at the above URL.

FWIW, you hit on my own selfish reasons for using "idx" instead of "i": 
easier searching (my XEmacs config highlights search terms in red) and 
greater "understandability".

-- 
paul moore
linux @ hp


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [patch] SELinux: trivial, unify iterator variable naming

[Vesa-Matti J Kari]

Hello,

On Wed, 30 Jul 2008, Paul Moore wrote:

> On Tuesday 29 July 2008 10:47:59 pm Russell Coker wrote:

> >
> > But using idx, index, or even names such as "count" which refer to
> > the meaning of the program might be more efficient overall.  I've
> > posted some more thoughts on this issue at the above URL.
>
> FWIW, you hit on my own selfish reasons for using "idx" instead of "i":
> easier searching (my XEmacs config highlights search terms in red) and
> greater "understandability".

I wish to express my disagreement, but please do not take this as a
request to alter the earlier patch rejection; I have no problem with it.

When using local variables as loop indexes or something equally
elementary, I always prefer "i" to "idx", "index" or "count". My rationale
is the following:

1) The existing convention; I've seen C-code like:

  for (i = 0; i < MAX; i++)

so many times that the "i" has almost become a part of the language
syntax in my brain. I'd guess many C-programmers feel the same way.

Granted, the "this is how it's always been" is a weak argument to
support anything, but personally, I gain no more insight from the
longer forms, so I prefer the more terse "i".

2) Related to the first; the existing code base. If I see 999, 99, or even
   9 functions in the same program/module consistenly using "int i", I
   will under no circumstance use "idx" in my additional function, but
   insist that all the functions use the same variable naming scheme.

   If, for some reason, the dominant variable naming for a particular code
   base happens to be "idx", "index", or "count", then I will adapt to
   that style.

   Further, I always to try extend the adaptation principle to the
   positioning of braces, whitespace, etc. so that all the code looks
   as if written by one person (I mean style-wise).

3) As Russell says in his blog: "Of course for some uses (such as a loop
   counter) there is little benefit in using grep." Yes. I also fail to
   see the drawbacks of using "FILE *f" inside a single function. Granted,
   if the functions are 3000-line monsters (yeah, unfortunately those do
   exist), you might have to use a more descriptive naming, but for the
   saner lengths, I am not so sure. For monster functions, refactoring
   it to smaller chunnks is usually the correct solution anyway, not
   changing "FILE *f" to "FILE *configFile".

4) Related to the particular case I addressed in my original message,
   the Linux Kernel Documentation/CodingStyle says:

  "LOCAL variable names should be short, and to the point.  If you have
  some random integer loop counter, it should probably be called "i".
  Calling it "loop_counter" is non-productive, if there is no chance of it
  being mis-understood.  Similarly, "tmp" can be just about any type of
  variable that is used to hold a temporary value."

Best regards,
vmk
-- 
************************************************************************
               Tietotekniikkaosasto / Helsingin yliopisto
                 IT Department / University of Helsinki
************************************************************************

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.