2002-07-22 10:57:03

by Marcin Dalecki

[permalink] [raw]
Subject: [PATCH] 2.5.27 enum

diff -urN linux-2.5.27/arch/i386/kernel/cpu/centaur.c linux/arch/i386/kernel/cpu/centaur.c
--- linux-2.5.27/arch/i386/kernel/cpu/centaur.c 2002-07-20 21:11:07.000000000 +0200
+++ linux/arch/i386/kernel/cpu/centaur.c 2002-07-21 19:30:42.000000000 +0200
@@ -267,7 +267,7 @@
DNA=1<<15,
ERETSTK=1<<16,
E2MMX=1<<19,
- EAMD3D=1<<20,
+ EAMD3D=1<<20
};

char *name;
diff -urN linux-2.5.27/arch/i386/kernel/mtrr.c linux/arch/i386/kernel/mtrr.c
--- linux-2.5.27/arch/i386/kernel/mtrr.c 2002-07-20 21:11:06.000000000 +0200
+++ linux/arch/i386/kernel/mtrr.c 2002-07-21 19:30:42.000000000 +0200
@@ -299,7 +299,7 @@
MTRR_IF_INTEL, /* Intel (P6) standard MTRRs */
MTRR_IF_AMD_K6, /* AMD pre-Athlon MTRRs */
MTRR_IF_CYRIX_ARR, /* Cyrix ARRs */
- MTRR_IF_CENTAUR_MCR, /* Centaur MCRs */
+ MTRR_IF_CENTAUR_MCR /* Centaur MCRs */
} mtrr_if = MTRR_IF_NONE;

static __initdata char *mtrr_if_name[] = {
diff -urN linux-2.5.27/fs/proc/base.c linux/fs/proc/base.c
--- linux-2.5.27/fs/proc/base.c 2002-07-20 21:11:07.000000000 +0200
+++ linux/fs/proc/base.c 2002-07-21 19:30:43.000000000 +0200
@@ -54,7 +54,7 @@
PROC_PID_MAPS,
PROC_PID_CPU,
PROC_PID_MOUNTS,
- PROC_PID_FD_DIR = 0x8000, /* 0x8000-0xffff */
+ PROC_PID_FD_DIR = 0x8000 /* 0x8000-0xffff */
};

struct pid_entry {
diff -urN linux-2.5.27/include/linux/backing-dev.h linux/include/linux/backing-dev.h
--- linux-2.5.27/include/linux/backing-dev.h 2002-07-20 21:11:12.000000000 +0200
+++ linux/include/linux/backing-dev.h 2002-07-21 19:30:43.000000000 +0200
@@ -13,7 +13,7 @@
*/
enum bdi_state {
BDI_pdflush, /* A pdflush thread is working this device */
- BDI_unused, /* Available bits start here */
+ BDI_unused /* Available bits start here */
};

struct backing_dev_info {
diff -urN linux-2.5.27/include/linux/blkdev.h linux/include/linux/blkdev.h
--- linux-2.5.27/include/linux/blkdev.h 2002-07-20 21:11:11.000000000 +0200
+++ linux/include/linux/blkdev.h 2002-07-21 19:30:43.000000000 +0200
@@ -87,7 +87,7 @@

__REQ_SPECIAL, /* driver suplied command */

- __REQ_NR_BITS, /* stops here */
+ __REQ_NR_BITS /* stops here */
};

#define REQ_RW (1 << __REQ_RW)
@@ -117,7 +117,7 @@

enum blk_queue_state {
Queue_down,
- Queue_up,
+ Queue_up
};

#define BLK_TAGS_PER_LONG (sizeof(unsigned long) * 8)
diff -urN linux-2.5.27/include/linux/buffer_head.h linux/include/linux/buffer_head.h
--- linux-2.5.27/include/linux/buffer_head.h 2002-07-20 21:11:06.000000000 +0200
+++ linux/include/linux/buffer_head.h 2002-07-21 19:30:43.000000000 +0200
@@ -24,7 +24,7 @@
BH_Async_Write, /* Is under end_buffer_async_write I/O */

BH_Boundary, /* Block is followed by a discontiguity */
- BH_PrivateStart,/* not a state bit, but the first bit available
+ BH_PrivateStart /* not a state bit, but the first bit available
* for private allocation by other entities
*/
};
diff -urN linux-2.5.27/include/linux/device.h linux/include/linux/device.h
--- linux-2.5.27/include/linux/device.h 2002-07-20 21:11:13.000000000 +0200
+++ linux/include/linux/device.h 2002-07-21 19:30:43.000000000 +0200
@@ -39,13 +39,13 @@
SUSPEND_NOTIFY,
SUSPEND_SAVE_STATE,
SUSPEND_DISABLE,
- SUSPEND_POWER_DOWN,
+ SUSPEND_POWER_DOWN
};

enum {
RESUME_POWER_ON,
RESUME_RESTORE_STATE,
- RESUME_ENABLE,
+ RESUME_ENABLE
};

struct device;
diff -urN linux-2.5.27/include/linux/genhd.h linux/include/linux/genhd.h
--- linux-2.5.27/include/linux/genhd.h 2002-07-20 21:11:09.000000000 +0200
+++ linux/include/linux/genhd.h 2002-07-21 22:15:00.000000000 +0200
@@ -38,7 +38,7 @@
/* Ours is not to wonder why.. */
BSD_PARTITION = FREEBSD_PARTITION,
MINIX_PARTITION = 0x81, /* Minix Partition ID */
- UNIXWARE_PARTITION = 0x63, /* Partition ID, same as */
+ UNIXWARE_PARTITION = 0x63 /* Partition ID, same as */
/* GNU_HURD and SCO Unix */
};

diff -urN linux-2.5.27/include/linux/jbd.h linux/include/linux/jbd.h
--- linux-2.5.27/include/linux/jbd.h 2002-07-20 21:11:33.000000000 +0200
+++ linux/include/linux/jbd.h 2002-07-21 19:30:43.000000000 +0200
@@ -226,13 +226,12 @@
#endif /* JBD_ASSERTIONS */

enum jbd_state_bits {
- BH_JBD /* Has an attached ext3 journal_head */
- = BH_PrivateStart,
+ BH_JBD = BH_PrivateStart, /* Has an attached ext3 journal_head */
BH_JWrite, /* Being written to log (@@@ DEBUGGING) */
BH_Freed, /* Has been freed (truncated) */
BH_Revoked, /* Has been revoked from the log */
BH_RevokeValid, /* Revoked flag is valid */
- BH_JBDDirty, /* Is dirty but journaled */
+ BH_JBDDirty /* Is dirty but journaled */
};

BUFFER_FNS(JBD, jbd)
diff -urN linux-2.5.27/include/linux/netdevice.h linux/include/linux/netdevice.h
--- linux-2.5.27/include/linux/netdevice.h 2002-07-20 21:12:22.000000000 +0200
+++ linux/include/linux/netdevice.h 2002-07-21 19:30:43.000000000 +0200
@@ -688,7 +688,7 @@
NETIF_MSG_INTR = 0x0200,
NETIF_MSG_TX_DONE = 0x0400,
NETIF_MSG_RX_STATUS = 0x0800,
- NETIF_MSG_PKTDATA = 0x1000,
+ NETIF_MSG_PKTDATA = 0x1000
};

#define netif_msg_drv(p) ((p)->msg_enable & NETIF_MSG_DRV)
diff -urN linux-2.5.27/include/linux/netfilter_ipv4.h linux/include/linux/netfilter_ipv4.h
--- linux-2.5.27/include/linux/netfilter_ipv4.h 2002-07-20 21:11:33.000000000 +0200
+++ linux/include/linux/netfilter_ipv4.h 2002-07-21 19:30:43.000000000 +0200
@@ -56,7 +56,7 @@
NF_IP_PRI_NAT_DST = -100,
NF_IP_PRI_FILTER = 0,
NF_IP_PRI_NAT_SRC = 100,
- NF_IP_PRI_LAST = INT_MAX,
+ NF_IP_PRI_LAST = INT_MAX
};

/* Arguments for setsockopt SOL_IP: */
diff -urN linux-2.5.27/include/linux/personality.h linux/include/linux/personality.h
--- linux-2.5.27/include/linux/personality.h 2002-07-20 21:11:21.000000000 +0200
+++ linux/include/linux/personality.h 2002-07-21 19:30:43.000000000 +0200
@@ -33,7 +33,7 @@
ADDR_LIMIT_32BIT = 0x0800000,
SHORT_INODE = 0x1000000,
WHOLE_SECONDS = 0x2000000,
- STICKY_TIMEOUTS = 0x4000000,
+ STICKY_TIMEOUTS = 0x4000000
};

/*
@@ -62,7 +62,7 @@
PER_RISCOS = 0x000c,
PER_SOLARIS = 0x000d | STICKY_TIMEOUTS,
PER_UW7 = 0x000e | STICKY_TIMEOUTS | MMAP_PAGE_ZERO,
- PER_MASK = 0x00ff,
+ PER_MASK = 0x00ff
};


diff -urN linux-2.5.27/include/linux/pkt_cls.h linux/include/linux/pkt_cls.h
--- linux-2.5.27/include/linux/pkt_cls.h 2002-07-20 21:11:06.000000000 +0200
+++ linux/include/linux/pkt_cls.h 2002-07-21 19:30:43.000000000 +0200
@@ -48,7 +48,7 @@
TCA_U32_LINK,
TCA_U32_DIVISOR,
TCA_U32_SEL,
- TCA_U32_POLICE,
+ TCA_U32_POLICE
};

#define TCA_U32_MAX TCA_U32_POLICE
@@ -96,7 +96,7 @@
TCA_RSVP_DST,
TCA_RSVP_SRC,
TCA_RSVP_PINFO,
- TCA_RSVP_POLICE,
+ TCA_RSVP_POLICE
};

#define TCA_RSVP_MAX TCA_RSVP_POLICE
@@ -126,7 +126,7 @@
TCA_ROUTE4_TO,
TCA_ROUTE4_FROM,
TCA_ROUTE4_IIF,
- TCA_ROUTE4_POLICE,
+ TCA_ROUTE4_POLICE
};

#define TCA_ROUTE4_MAX TCA_ROUTE4_POLICE
@@ -138,7 +138,7 @@
{
TCA_FW_UNSPEC,
TCA_FW_CLASSID,
- TCA_FW_POLICE,
+ TCA_FW_POLICE
};

#define TCA_FW_MAX TCA_FW_POLICE
@@ -153,7 +153,7 @@
TCA_TCINDEX_SHIFT,
TCA_TCINDEX_FALL_THROUGH,
TCA_TCINDEX_CLASSID,
- TCA_TCINDEX_POLICE,
+ TCA_TCINDEX_POLICE
};

#define TCA_TCINDEX_MAX TCA_TCINDEX_POLICE
diff -urN linux-2.5.27/include/linux/pkt_sched.h linux/include/linux/pkt_sched.h
--- linux-2.5.27/include/linux/pkt_sched.h 2002-07-20 21:11:29.000000000 +0200
+++ linux/include/linux/pkt_sched.h 2002-07-21 19:30:43.000000000 +0200
@@ -128,7 +128,7 @@
TCA_CSZ_UNSPEC,
TCA_CSZ_PARMS,
TCA_CSZ_RTAB,
- TCA_CSZ_PTAB,
+ TCA_CSZ_PTAB
};

/* TBF section */
@@ -147,7 +147,7 @@
TCA_TBF_UNSPEC,
TCA_TBF_PARMS,
TCA_TBF_RTAB,
- TCA_TBF_PTAB,
+ TCA_TBF_PTAB
};


@@ -181,7 +181,7 @@
{
TCA_RED_UNSPEC,
TCA_RED_PARMS,
- TCA_RED_STAB,
+ TCA_RED_STAB
};

struct tc_red_qopt
@@ -213,7 +213,7 @@
TCA_GRED_UNSPEC,
TCA_GRED_PARMS,
TCA_GRED_STAB,
- TCA_GRED_DPS,
+ TCA_GRED_DPS
};

#define TCA_SET_OFF TCA_GRED_PARMS
@@ -327,7 +327,7 @@
TCA_CBQ_OVL_STRATEGY,
TCA_CBQ_RATE,
TCA_CBQ_RTAB,
- TCA_CBQ_POLICE,
+ TCA_CBQ_POLICE
};

#define TCA_CBQ_MAX TCA_CBQ_POLICE
diff -urN linux-2.5.27/include/linux/pm.h linux/include/linux/pm.h
--- linux-2.5.27/include/linux/pm.h 2002-07-20 21:11:24.000000000 +0200
+++ linux/include/linux/pm.h 2002-07-21 19:30:43.000000000 +0200
@@ -45,7 +45,7 @@

/* base station management */
PM_EJECT,
- PM_LOCK,
+ PM_LOCK
};

typedef int pm_request_t;
@@ -61,7 +61,7 @@
PM_USB_DEV, /* USB device */
PM_SCSI_DEV, /* SCSI device */
PM_ISA_DEV, /* ISA device */
- PM_MTD_DEV, /* Memory Technology Device */
+ PM_MTD_DEV /* Memory Technology Device */
};

typedef int pm_dev_t;
@@ -77,7 +77,7 @@
PM_SYS_IRDA = 0x41d00510, /* IRDA controller */
PM_SYS_FDC = 0x41d00700, /* floppy controller */
PM_SYS_VGA = 0x41d00900, /* VGA controller */
- PM_SYS_PCMCIA = 0x41d00e00, /* PCMCIA controller */
+ PM_SYS_PCMCIA = 0x41d00e00 /* PCMCIA controller */
};

/*
diff -urN linux-2.5.27/include/linux/proc_fs.h linux/include/linux/proc_fs.h
--- linux-2.5.27/include/linux/proc_fs.h 2002-07-20 21:11:03.000000000 +0200
+++ linux/include/linux/proc_fs.h 2002-07-21 19:30:43.000000000 +0200
@@ -21,7 +21,7 @@
*/

enum {
- PROC_ROOT_INO = 1,
+ PROC_ROOT_INO = 1
};

/* Finally, the dynamically allocatable proc entries are reserved: */
diff -urN linux-2.5.27/include/linux/root_dev.h linux/include/linux/root_dev.h
--- linux-2.5.27/include/linux/root_dev.h 2002-07-20 21:11:11.000000000 +0200
+++ linux/include/linux/root_dev.h 2002-07-21 19:30:43.000000000 +0200
@@ -11,7 +11,7 @@
Root_SDA1 = MKDEV(SCSI_DISK0_MAJOR, 1),
Root_SDA2 = MKDEV(SCSI_DISK0_MAJOR, 2),
Root_HDC1 = MKDEV(IDE1_MAJOR, 1),
- Root_SR0 = MKDEV(SCSI_CDROM_MAJOR, 0),
+ Root_SR0 = MKDEV(SCSI_CDROM_MAJOR, 0)
};

extern dev_t ROOT_DEV;
diff -urN linux-2.5.27/include/linux/rtnetlink.h linux/include/linux/rtnetlink.h
--- linux-2.5.27/include/linux/rtnetlink.h 2002-07-20 21:11:32.000000000 +0200
+++ linux/include/linux/rtnetlink.h 2002-07-21 19:30:43.000000000 +0200
@@ -112,7 +112,7 @@
RTN_PROHIBIT, /* Administratively prohibited */
RTN_THROW, /* Not in this table */
RTN_NAT, /* Translate this address */
- RTN_XRESOLVE, /* Use external resolver */
+ RTN_XRESOLVE /* Use external resolver */
};

#define RTN_MAX RTN_XRESOLVE
@@ -278,7 +278,7 @@
#define RTAX_CWND RTAX_CWND
RTAX_ADVMSS,
#define RTAX_ADVMSS RTAX_ADVMSS
- RTAX_REORDERING,
+ RTAX_REORDERING
#define RTAX_REORDERING RTAX_REORDERING
};

@@ -442,7 +442,7 @@
#define IFLA_PRIORITY IFLA_PRIORITY
IFLA_MASTER,
#define IFLA_MASTER IFLA_MASTER
- IFLA_WIRELESS, /* Wireless Extension event - see wireless.h */
+ IFLA_WIRELESS /* Wireless Extension event - see wireless.h */
#define IFLA_WIRELESS IFLA_WIRELESS
};

@@ -503,7 +503,7 @@
TCA_OPTIONS,
TCA_STATS,
TCA_XSTATS,
- TCA_RATE,
+ TCA_RATE
};

#define TCA_MAX TCA_RATE
diff -urN linux-2.5.27/include/linux/sunrpc/debug.h linux/include/linux/sunrpc/debug.h
--- linux-2.5.27/include/linux/sunrpc/debug.h 2002-07-20 21:11:14.000000000 +0200
+++ linux/include/linux/sunrpc/debug.h 2002-07-21 19:30:43.000000000 +0200
@@ -89,7 +89,7 @@
CTL_RPCDEBUG = 1,
CTL_NFSDEBUG,
CTL_NFSDDEBUG,
- CTL_NLMDEBUG,
+ CTL_NLMDEBUG
};

#endif /* _LINUX_SUNRPC_DEBUG_H_ */
diff -urN linux-2.5.27/include/linux/sunrpc/msg_prot.h linux/include/linux/sunrpc/msg_prot.h
--- linux-2.5.27/include/linux/sunrpc/msg_prot.h 2002-07-20 21:11:10.000000000 +0200
+++ linux/include/linux/sunrpc/msg_prot.h 2002-07-21 19:30:43.000000000 +0200
@@ -16,7 +16,7 @@
RPC_AUTH_UNIX = 1,
RPC_AUTH_SHORT = 2,
RPC_AUTH_DES = 3,
- RPC_AUTH_KRB = 4,
+ RPC_AUTH_KRB = 4
};

enum rpc_msg_type {
diff -urN linux-2.5.27/include/linux/swap.h linux/include/linux/swap.h
--- linux-2.5.27/include/linux/swap.h 2002-07-20 21:11:06.000000000 +0200
+++ linux/include/linux/swap.h 2002-07-21 19:30:43.000000000 +0200
@@ -89,7 +89,7 @@
enum {
SWP_USED = (1 << 0), /* is slot in swap_info[] used? */
SWP_WRITEOK = (1 << 1), /* ok to write to this swap? */
- SWP_ACTIVE = (SWP_USED | SWP_WRITEOK),
+ SWP_ACTIVE = (SWP_USED | SWP_WRITEOK)
};

#define SWAP_CLUSTER_MAX 32
diff -urN linux-2.5.27/include/linux/tcp_diag.h linux/include/linux/tcp_diag.h
--- linux-2.5.27/include/linux/tcp_diag.h 2002-07-20 21:11:23.000000000 +0200
+++ linux/include/linux/tcp_diag.h 2002-07-21 19:30:43.000000000 +0200
@@ -34,7 +34,7 @@
enum
{
TCPDIAG_REQ_NONE,
- TCPDIAG_REQ_BYTECODE,
+ TCPDIAG_REQ_BYTECODE
};

#define TCPDIAG_REQ_MAX TCPDIAG_REQ_BYTECODE
@@ -62,7 +62,7 @@
TCPDIAG_BC_D_LE,
TCPDIAG_BC_AUTO,
TCPDIAG_BC_S_COND,
- TCPDIAG_BC_D_COND,
+ TCPDIAG_BC_D_COND
};

struct tcpdiag_hostcond
@@ -97,7 +97,7 @@
{
TCPDIAG_NONE,
TCPDIAG_MEMINFO,
- TCPDIAG_INFO,
+ TCPDIAG_INFO
};

#define TCPDIAG_MAX TCPDIAG_INFO
diff -urN linux-2.5.27/include/linux/writeback.h linux/include/linux/writeback.h
--- linux-2.5.27/include/linux/writeback.h 2002-07-20 21:11:08.000000000 +0200
+++ linux/include/linux/writeback.h 2002-07-21 19:30:43.000000000 +0200
@@ -28,7 +28,7 @@
WB_SYNC_NONE = 0, /* Don't wait on anything */
WB_SYNC_LAST = 1, /* Wait on the last-written mapping */
WB_SYNC_ALL = 2, /* Wait on every mapping */
- WB_SYNC_HOLD = 3, /* Hold the inode on sb_dirty for sys_sync() */
+ WB_SYNC_HOLD = 3 /* Hold the inode on sb_dirty for sys_sync() */
};

void writeback_unlocked_inodes(int *nr_to_write,


Attachments:
enum-2.5.27.diff (13.05 kB)

2002-07-22 19:58:10

by Benjamin LaHaise

[permalink] [raw]
Subject: Re: [PATCH] 2.5.27 enum

On Mon, Jul 22, 2002 at 12:53:21PM +0200, Marcin Dalecki wrote:
> - Fix a bunch of places where there are trailing "," at the
> end of enum declarations.

Please don't apply this. By leaving the trailing "," on enums, additional
values can be added by merely inserting an additional + line in a patch,
otherwise there are excess conflicts when multiple patches add values to
the enum.

-ben

2002-07-23 02:19:18

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] 2.5.27 enum

From: Benjamin LaHaise <[email protected]>
Date: Mon, 22 Jul 2002 16:01:18 -0400

Please don't apply this. By leaving the trailing "," on enums, additional
values can be added by merely inserting an additional + line in a patch,
otherwise there are excess conflicts when multiple patches add values to
the enum.

I totally agree.

What is the purpose of all of these zany patches? Are you going to
remove the inline assembler from the whole tree too? :-)

2002-07-23 02:52:31

by Alexander Viro

[permalink] [raw]
Subject: Re: [PATCH] 2.5.27 enum



On Mon, 22 Jul 2002, David S. Miller wrote:

> I totally agree.
>
> What is the purpose of all of these zany patches?

Harry: Well, a couple of things...
King: Correct, and one of those things is...?
Harry: Best not mentioned, really.
King: Right! And the other is fornication!

(with apologies to Richard Curtis and Rowan Atkinson...)

2002-07-23 12:23:56

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] 2.5.27 enum

On Mon, Jul 22, 2002 at 04:01:18PM -0400, Benjamin LaHaise wrote:
> On Mon, Jul 22, 2002 at 12:53:21PM +0200, Marcin Dalecki wrote:
> > - Fix a bunch of places where there are trailing "," at the
> > end of enum declarations.
>
> Please don't apply this. By leaving the trailing "," on enums, additional
> values can be added by merely inserting an additional + line in a patch,
> otherwise there are excess conflicts when multiple patches add values to
> the enum.

Gratuitous 'cleanups' with no real redeeming feature also have another
downside which a lot of people seem to overlook. They completely screws
over anyone who also has a pending patch in that area if Linus applies it.

For most people this is five minutes work as they fix up by hand
the single reject in one or two places. For people like myself keeping
a large patchset, this is a lot of extra work for absolutely no gain.
Two kernels later, someone adds a new sysctl which re-adds the , at
the end anyway.

We have much bigger problems to fix than silly[1] things like this.

Dave

[1] Maybe silly is the wrong word to use, but I didn't want to use
'trivial' for fear of putting down the usefulness of Rusty's
trivial patches.

--
| Dave Jones. http://www.codemonkey.org.uk
| SuSE Labs

2002-07-23 12:43:33

by Marcin Dalecki

[permalink] [raw]
Subject: Re: [PATCH] 2.5.27 enum

Dave Jones wrote:
> On Mon, Jul 22, 2002 at 04:01:18PM -0400, Benjamin LaHaise wrote:
> > On Mon, Jul 22, 2002 at 12:53:21PM +0200, Marcin Dalecki wrote:
> > > - Fix a bunch of places where there are trailing "," at the
> > > end of enum declarations.
> >
> > Please don't apply this. By leaving the trailing "," on enums, additional
> > values can be added by merely inserting an additional + line in a patch,
> > otherwise there are excess conflicts when multiple patches add values to
> > the enum.
>
> Gratuitous 'cleanups' with no real redeeming feature also have another
> downside which a lot of people seem to overlook. They completely screws
> over anyone who also has a pending patch in that area if Linus applies it.
>
> For most people this is five minutes work as they fix up by hand
> the single reject in one or two places. For people like myself keeping
> a large patchset, this is a lot of extra work for absolutely no gain.
> Two kernels later, someone adds a new sysctl which re-adds the , at
> the end anyway.
>
> We have much bigger problems to fix than silly[1] things like this.

Enabling -pedantic spotted me at least immediately at the
bug in readv/writev fixed in the same series of kernel without
resorting to LSB testing as Alan explained how he came across this
botch. So it's not entierly futile.

But for the enum case I agree that GCC is hossed and simply shouldn't
warn about it if in C99 mode. Personally I was just still thinking at
C89 level. Well after all I already fixed this in the GCC I use.

And no matter what - there is not much working in the sysctl area and
non sized array forward declarations are not a nice thing both: to read
and to the compiler. Same applies to the gratitious macro or ({ })
overusages in the other patches. Inlined functions how the advantage of
1. stricter type checking.
2. Possibly faster compilation(if only included by files which really
use them of course)



Subject: Re: [PATCH] 2.5.27 enum


On Tue, 23 Jul 2002, Dave Jones wrote:

> On Mon, Jul 22, 2002 at 04:01:18PM -0400, Benjamin LaHaise wrote:
> > On Mon, Jul 22, 2002 at 12:53:21PM +0200, Marcin Dalecki wrote:
> > > - Fix a bunch of places where there are trailing "," at the
> > > end of enum declarations.
> >
> > Please don't apply this. By leaving the trailing "," on enums, additional
> > values can be added by merely inserting an additional + line in a patch,
> > otherwise there are excess conflicts when multiple patches add values to
> > the enum.
>
> Gratuitous 'cleanups' with no real redeeming feature also have another
> downside which a lot of people seem to overlook. They completely screws
> over anyone who also has a pending patch in that area if Linus applies it.
>
> For most people this is five minutes work as they fix up by hand
> the single reject in one or two places. For people like myself keeping
> a large patchset, this is a lot of extra work for absolutely no gain.
> Two kernels later, someone adds a new sysctl which re-adds the , at
> the end anyway.

Now imagine my pain keeping in sync with IDE 2.5 or even
reviewing IDE patches...

Regards
--
Bartlomiej

> We have much bigger problems to fix than silly[1] things like this.
>
> Dave
>
> [1] Maybe silly is the wrong word to use, but I didn't want to use
> 'trivial' for fear of putting down the usefulness of Rusty's
> trivial patches.
>
> --
> | Dave Jones. http://www.codemonkey.org.uk
> | SuSE Labs
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2002-07-24 06:41:51

by James Cloos

[permalink] [raw]
Subject: Re: [PATCH] 2.5.27 enum

bcrl> Please don't apply this. By leaving the trailing "," on enums,
bcrl> additional values can be added by merely inserting an additional
bcrl> + line in a patch, otherwise there are excess conflicts when
bcrl> multiple patches add values to the enum.

davem> I totally agree.

Is my memory hosed or was there some years back a patch that
specifically *added* the trailing commas to the tree, for the express
reasons Ben mentions above?

-JimC

2002-07-24 08:39:27

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] 2.5.27 enum

On Tue, 23 Jul 2002 14:27:04 +0200
Dave Jones <[email protected]> wrote:

> On Mon, Jul 22, 2002 at 04:01:18PM -0400, Benjamin LaHaise wrote:
> > On Mon, Jul 22, 2002 at 12:53:21PM +0200, Marcin Dalecki wrote:
> > > - Fix a bunch of places where there are trailing "," at the
> > > end of enum declarations.
> >
> > Please don't apply this. By leaving the trailing "," on enums, additional
> > values can be added by merely inserting an additional + line in a patch,
> > otherwise there are excess conflicts when multiple patches add values to
> > the enum.
>
> Gratuitous 'cleanups' with no real redeeming feature also have another
> downside which a lot of people seem to overlook. They completely screws
> over anyone who also has a pending patch in that area if Linus applies it.

Yes. It particularly sucks on the "maintainerless" core code which is always
in flux. This is also why I generally reject whitespace-cleanup patches,
and originally rejected the "doesnt" patches (I got convinced by the pedants).

OTOH, 90% of kernel code is copied from elsewhere, so janitorial cleanups
*are* worthwhile, as long as they are one-liners, or fix a real problem.

Rusty.
--
there are those who do and those who hang on and you don't see too
many doers quoting their contemporaries. -- Larry McVoy

2002-07-24 09:44:53

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] 2.5.27 enum

On Wed, Jul 24, 2002 at 02:49:37PM +1000, Rusty Russell wrote:
> Yes. It particularly sucks on the "maintainerless" core code which is always
> in flux. This is also why I generally reject whitespace-cleanup patches,
> and originally rejected the "doesnt" patches (I got convinced by the pedants).
>
> OTOH, 90% of kernel code is copied from elsewhere, so janitorial cleanups
> *are* worthwhile, as long as they are one-liners, or fix a real problem.

I agree in part. Take the initialiser patches you're currently carrying
for example. Whilst they're more useful (and more likely) to get merged
than the enum patches, they also have the annoying issue that anyone
currently working on code near those gets shafted.

With large touching patches like these, the only way to not piss people
off is to find out who's working on a particular area, and work with
them. "Can you roll this into your current working tree, and push to
Linus next time". Instead of just shovelling straight to Linus.
(Note, you did seem to actually seem to do the right thing here FWICS.
have a gold star to go alongside your recent black one).

Dave

--
| Dave Jones. http://www.codemonkey.org.uk
| SuSE Labs