netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: David Miller <davem@davemloft.net>
Cc: shhuiw@gmail.com, fubar@us.ibm.com, andy@greyhouse.net,
	netdev@vger.kernel.org
Subject: Re: [PATCH] bonding: use pre-defined macro in bond_mode_name instead of magic number 0
Date: Fri, 26 Jul 2013 14:25:58 -0700	[thread overview]
Message-ID: <1374873958.2009.9.camel@joe-AO722> (raw)
In-Reply-To: <20130726.135330.232525167518786234.davem@davemloft.net>

On Fri, 2013-07-26 at 13:53 -0700, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Wed, 24 Jul 2013 00:10:19 -0700
> 
> > Probably be simpler, less confusing, and more normal style
> > to use a switch case.
> > 
> >       switch (mode) {
> >       case BOND_MODE_ROUNDROBIN:
> >               return "load balancing (round-robin)";
> >       case BOND_MODE_ACTIVEBACKUP:
> >               return "fault-tolerance (active-backup)";
> >       case BOND_MODE_XOR:
> >               return "load balancing (xor)";
> >       case BOND_MODE_BROADCAST;
> >               return "fault-tolerance (broadcast)";
> >       case BOND_MODE_8023AD:
> >               return "IEEE 802.3ad Dynamic link aggregation";
> >       case BOND_MODE_TLB:
> >               return "transmit load balancing";
> >       case BOND_MODE_ALB:
> >               return "adaptive load balancing";
> >       default:
> >               return "unknown";
> >       }
> 
> I have to say that I do not prefer this approach.
> 
> I have a good idea what the compiler ends up outputting for the above,
> and the "indexed table of strings" approach is much less code.
> 

just fyi: it doesn't seem like "much less" to at least gcc 4.7

$ cat t1.c
#include <stdio.h>
#include <stdlib.h>

const char * __attribute__((noinline)) string1(int index)
{
  static const char *strings[] = {
    [0] = "String10",
    [1] = "String11",
    [2] = "String12",
    [3] = "String13",
    [4] = "String14",
    [5] = "String15",
  };
  if (index < 0 || index > 5)
    return NULL;
  return strings[index];
}

const char * __attribute__((noinline)) string2(int index)
{
  switch (index) {
  case 0: return "String20";
  case 1: return "String21";
  case 2: return "String22";
  case 3: return "String23";
  case 4: return "String24";
  case 5: return "String25";
  }
  return NULL;
}

int main(int argc, char** argv)
{
  int val;

  srand(time(NULL));
  val = rand()%6;
  printf("1: %s, 2: %s\n", string1(val), string2(val));
  return 0;
}
$ gcc -S -O2 t1.c
$ cat t1.s
	.file	"t1.c"
	.text
	.p2align 4,,15
	.globl	string1
	.type	string1, @function
string1:
.LFB38:
	.cfi_startproc
	movl	4(%esp), %eax
	cmpl	$5, %eax
	ja	.L3
	movl	strings.2633(,%eax,4), %eax
	ret
	.p2align 4,,7
	.p2align 3
.L3:
	xorl	%eax, %eax
	ret
	.cfi_endproc
.LFE38:
	.size	string1, .-string1
	.p2align 4,,15
	.globl	string2
	.type	string2, @function
string2:
.LFB39:
	.cfi_startproc
	movl	4(%esp), %edx
	xorl	%eax, %eax
	cmpl	$5, %edx
	ja	.L6
	movl	CSWTCH.5(,%edx,4), %eax
.L6:
	rep
	ret
	.cfi_endproc
.LFE39:
	.size	string2, .-string2
	.section	.rodata.str1.1,"aMS",@progbits,1
.LC0:
	.string	"1: %s, 2: %s\n"
	.section	.text.startup,"ax",@progbits
	.p2align 4,,15
	.globl	main
	.type	main, @function
main:
.LFB40:
	.cfi_startproc
	pushl	%ebp
	.cfi_def_cfa_offset 8
	.cfi_offset 5, -8
	movl	%esp, %ebp
	.cfi_def_cfa_register 5
	pushl	%esi
	pushl	%ebx
	.cfi_offset 6, -12
	.cfi_offset 3, -16
	movl	$715827883, %ebx
	andl	$-16, %esp
	subl	$16, %esp
	movl	$0, (%esp)
	call	time
	movl	%eax, (%esp)
	call	srand
	call	rand
	movl	%eax, %ecx
	imull	%ebx
	movl	%ecx, %eax
	sarl	$31, %eax
	movl	%edx, %ebx
	subl	%eax, %ebx
	leal	(%ebx,%ebx,2), %eax
	movl	%ecx, %ebx
	addl	%eax, %eax
	subl	%eax, %ebx
	movl	%ebx, (%esp)
	call	string2
	movl	%ebx, (%esp)
	movl	%eax, %esi
	call	string1
	movl	%esi, 12(%esp)
	movl	$.LC0, 4(%esp)
	movl	$1, (%esp)
	movl	%eax, 8(%esp)
	call	__printf_chk
	leal	-8(%ebp), %esp
	xorl	%eax, %eax
	popl	%ebx
	.cfi_restore 3
	popl	%esi
	.cfi_restore 6
	popl	%ebp
	.cfi_restore 5
	.cfi_def_cfa 4, 4
	ret
	.cfi_endproc
.LFE40:
	.size	main, .-main
	.section	.rodata.str1.1
.LC1:
	.string	"String10"
.LC2:
	.string	"String11"
.LC3:
	.string	"String12"
.LC4:
	.string	"String13"
.LC5:
	.string	"String14"
.LC6:
	.string	"String15"
	.section	.rodata
	.align 4
	.type	strings.2633, @object
	.size	strings.2633, 24
strings.2633:
	.long	.LC1
	.long	.LC2
	.long	.LC3
	.long	.LC4
	.long	.LC5
	.long	.LC6
	.section	.rodata.str1.1
.LC7:
	.string	"String20"
.LC8:
	.string	"String21"
.LC9:
	.string	"String22"
.LC10:
	.string	"String23"
.LC11:
	.string	"String24"
.LC12:
	.string	"String25"
	.section	.rodata
	.align 4
	.type	CSWTCH.5, @object
	.size	CSWTCH.5, 24
CSWTCH.5:
	.long	.LC7
	.long	.LC8
	.long	.LC9
	.long	.LC10
	.long	.LC11
	.long	.LC12
	.ident	"GCC: (Ubuntu/Linaro 4.7.3-1ubuntu1) 4.7.3"
	.section	.note.GNU-stack,"",@progbits

  parent reply	other threads:[~2013-07-26 21:26 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-24  6:53 [PATCH] bonding: use pre-defined macro in bond_mode_name instead of magic number 0 Wang Sheng-Hui
2013-07-24  7:10 ` Joe Perches
2013-07-24 12:38   ` Wang Sheng-Hui
2013-07-26 20:53   ` David Miller
2013-07-26 20:56     ` Joe Perches
2013-07-26 21:25     ` Joe Perches [this message]
2013-07-26 20:54 ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1374873958.2009.9.camel@joe-AO722 \
    --to=joe@perches.com \
    --cc=andy@greyhouse.net \
    --cc=davem@davemloft.net \
    --cc=fubar@us.ibm.com \
    --cc=netdev@vger.kernel.org \
    --cc=shhuiw@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).