Cleanup whitespace and warn about wrong volatile usage. This code
loves to use deprecated if ((err = foo())), but I did not have enough
strength to fix all of it. Ouch also maintainers item would be handy.
Signed-off-by: Pavel Machek <[email protected]>
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 79d63c9..86064a8 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -252,6 +252,7 @@ enum {
struct azx_dev {
u32 *bdl; /* virtual address of the BDL */
dma_addr_t bdl_addr; /* physical address of the BDL */
+ /* FIXME: volatile does not provide needed locking on SMP systems */
volatile u32 *posbuf; /* position buffer pointer */
unsigned int bufsize; /* size of the play buffer in bytes */
@@ -271,8 +272,8 @@ struct azx_dev {
/* for sanity check of position buffer */
unsigned int period_intr;
- unsigned int opened: 1;
- unsigned int running: 1;
+ unsigned int opened :1;
+ unsigned int running :1;
};
/* CORB/RIRB */
@@ -330,8 +331,8 @@ struct azx {
/* flags */
int position_fix;
- unsigned int initialized: 1;
- unsigned int single_cmd: 1;
+ unsigned int initialized :1;
+ unsigned int single_cmd :1;
};
/* driver types */
@@ -642,14 +643,14 @@ static int azx_reset(struct azx *chip)
azx_writeb(chip, GCTL, azx_readb(chip, GCTL) | ICH6_GCTL_RESET);
count = 50;
- while (! azx_readb(chip, GCTL) && --count)
+ while (!azx_readb(chip, GCTL) && --count)
msleep(1);
- /* Brent Chartrand said to wait >= 540us for codecs to intialize */
+ /* Brent Chartrand said to wait >= 540us for codecs to initialize */
msleep(1);
/* check to see if controller is ready */
- if (! azx_readb(chip, GCTL)) {
+ if (!azx_readb(chip, GCTL)) {
snd_printd("azx_reset: controller not ready!\n");
return -EBUSY;
}
@@ -658,7 +659,7 @@ static int azx_reset(struct azx *chip)
azx_writel(chip, GCTL, azx_readl(chip, GCTL) | ICH6_GCTL_UREN);
/* detect codecs */
- if (! chip->codec_mask) {
+ if (!chip->codec_mask) {
chip->codec_mask = azx_readw(chip, STATESTS);
snd_printdd("codec_mask = 0x%x\n", chip->codec_mask);
}
@@ -766,7 +767,7 @@ static void azx_init_chip(struct azx *ch
azx_int_enable(chip);
/* initialize the codec command I/O */
- if (! chip->single_cmd)
+ if (!chip->single_cmd)
azx_init_cmd_io(chip);
/* program the position buffer */
@@ -794,7 +795,7 @@ static void azx_init_chip(struct azx *ch
/*
* interrupt handler
*/
-static irqreturn_t azx_interrupt(int irq, void* dev_id, struct pt_regs *regs)
+static irqreturn_t azx_interrupt(int irq, void *dev_id, struct pt_regs *regs)
{
struct azx *chip = dev_id;
struct azx_dev *azx_dev;
@@ -999,8 +1000,7 @@ static struct snd_pcm_hardware azx_pcm_h
.info = (SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_INTERLEAVED |
SNDRV_PCM_INFO_BLOCK_TRANSFER |
SNDRV_PCM_INFO_MMAP_VALID |
- SNDRV_PCM_INFO_PAUSE /*|*/
- /*SNDRV_PCM_INFO_RESUME*/),
+ SNDRV_PCM_INFO_PAUSE),
.formats = SNDRV_PCM_FMTBIT_S16_LE,
.rates = SNDRV_PCM_RATE_48000,
.rate_min = 48000,
@@ -1322,6 +1322,8 @@ static int __devinit azx_init_stream(str
* assign the starting bdl address to each stream (device) and initialize
*/
for (i = 0; i < chip->num_streams; i++) {
+ /* FIXME: this should probably use readl or something.
+ hand-doing volatiles is wrong. */
unsigned int off = sizeof(u32) * (i * AZX_MAX_FRAG * 4);
struct azx_dev *azx_dev = &chip->azx_dev[i];
azx_dev->bdl = (u32 *)(chip->bdl.area + off);
@@ -1399,6 +1401,7 @@ static int azx_free(struct azx *chip)
azx_writel(chip, DPUBASE, 0);
/* wait a little for interrupts to finish */
+ /* FIXME: delay is not right way to wait for interrupts */
msleep(1);
}
@@ -1434,19 +1437,19 @@ static int __devinit azx_create(struct s
struct azx **rchip)
{
struct azx *chip;
- int err = 0;
+ int err;
static struct snd_device_ops ops = {
.dev_free = azx_dev_free,
};
*rchip = NULL;
- if ((err = pci_enable_device(pci)) < 0)
+ err = pci_enable_device(pci);
+ if (err < 0)
return err;
chip = kzalloc(sizeof(*chip), GFP_KERNEL);
-
- if (NULL == chip) {
+ if (!chip) {
snd_printk(KERN_ERR SFX "cannot allocate chip\n");
pci_disable_device(pci);
return -ENOMEM;
@@ -1472,13 +1475,14 @@ static int __devinit azx_create(struct s
}
#endif
- if ((err = pci_request_regions(pci, "ICH HD audio")) < 0) {
+ err = pci_request_regions(pci, "ICH HD audio");
+ if (err < 0) {
kfree(chip);
pci_disable_device(pci);
return err;
}
- chip->addr = pci_resource_start(pci,0);
+ chip->addr = pci_resource_start(pci, 0);
chip->remap_addr = ioremap_nocache(chip->addr, pci_resource_len(pci,0));
if (chip->remap_addr == NULL) {
snd_printk(KERN_ERR SFX "ioremap error\n");
@@ -1519,7 +1523,7 @@ static int __devinit azx_create(struct s
}
chip->num_streams = chip->playback_streams + chip->capture_streams;
chip->azx_dev = kcalloc(chip->num_streams, sizeof(*chip->azx_dev), GFP_KERNEL);
- if (! chip->azx_dev) {
+ if (!chip->azx_dev) {
snd_printk(KERN_ERR "cannot malloc azx_dev\n");
goto errout;
}
@@ -1550,7 +1554,7 @@ static int __devinit azx_create(struct s
chip->initialized = 1;
/* codec detection */
- if (! chip->codec_mask) {
+ if (!chip->codec_mask) {
snd_printk(KERN_ERR SFX "no codecs found!\n");
err = -ENODEV;
goto errout;
@@ -1577,16 +1581,16 @@ static int __devinit azx_probe(struct pc
{
struct snd_card *card;
struct azx *chip;
- int err = 0;
+ int err;
card = snd_card_new(index, id, THIS_MODULE, 0);
- if (NULL == card) {
+ if (!card) {
snd_printk(KERN_ERR SFX "Error creating card!\n");
return -ENOMEM;
}
- if ((err = azx_create(card, pci, pci_id->driver_data,
- &chip)) < 0) {
+ err = azx_create(card, pci, pci_id->driver_data, &chip);
+ if (err < 0) {
snd_card_free(card);
return err;
}
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
At Thu, 31 Aug 2006 14:37:06 +0200,
Pavel Machek wrote:
>
> @@ -271,8 +272,8 @@ struct azx_dev {
> /* for sanity check of position buffer */
> unsigned int period_intr;
>
> - unsigned int opened: 1;
> - unsigned int running: 1;
> + unsigned int opened :1;
> + unsigned int running :1;
> };
>
> /* CORB/RIRB */
> @@ -330,8 +331,8 @@ struct azx {
>
> /* flags */
> int position_fix;
> - unsigned int initialized: 1;
> - unsigned int single_cmd: 1;
> + unsigned int initialized :1;
> + unsigned int single_cmd :1;
> };
Any official standard reference for bit-field expressions?
>
> /* driver types */
> @@ -642,14 +643,14 @@ static int azx_reset(struct azx *chip)
> azx_writeb(chip, GCTL, azx_readb(chip, GCTL) | ICH6_GCTL_RESET);
>
> count = 50;
> - while (! azx_readb(chip, GCTL) && --count)
> + while (!azx_readb(chip, GCTL) && --count)
> msleep(1);
Hm, it looks rather like a personal preference.
IMHO, it's harder to read without space...
> @@ -999,8 +1000,7 @@ static struct snd_pcm_hardware azx_pcm_h
> .info = (SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_INTERLEAVED |
> SNDRV_PCM_INFO_BLOCK_TRANSFER |
> SNDRV_PCM_INFO_MMAP_VALID |
> - SNDRV_PCM_INFO_PAUSE /*|*/
> - /*SNDRV_PCM_INFO_RESUME*/),
> + SNDRV_PCM_INFO_PAUSE),
The commented out item is there intentionally to indicate that the
full-resume isn't implemented.
I'll fix the volatile things separately.
Thanks,
Takashi
Hi!
> > @@ -271,8 +272,8 @@ struct azx_dev {
> > /* for sanity check of position buffer */
> > unsigned int period_intr;
> >
> > - unsigned int opened: 1;
> > - unsigned int running: 1;
> > + unsigned int opened :1;
> > + unsigned int running :1;
> > };
> >
> > /* CORB/RIRB */
> > @@ -330,8 +331,8 @@ struct azx {
> >
> > /* flags */
> > int position_fix;
> > - unsigned int initialized: 1;
> > - unsigned int single_cmd: 1;
> > + unsigned int initialized :1;
> > + unsigned int single_cmd :1;
> > };
>
> Any official standard reference for bit-field expressions?
Well, logically : belongs to the 1, and include/linux understands it
like that...
> > /* driver types */
> > @@ -642,14 +643,14 @@ static int azx_reset(struct azx *chip)
> > azx_writeb(chip, GCTL, azx_readb(chip, GCTL) | ICH6_GCTL_RESET);
> >
> > count = 50;
> > - while (! azx_readb(chip, GCTL) && --count)
> > + while (!azx_readb(chip, GCTL) && --count)
> > msleep(1);
>
> Hm, it looks rather like a personal preference.
> IMHO, it's harder to read without space...
Well, core parts (sched.c?) use it without the space, and I'd say (!
expression) is unusual in kernel, but no, could not find it codified.
> I'll fix the volatile things separately.
Thanks.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
At Thu, 31 Aug 2006 15:39:29 +0200,
Pavel Machek wrote:
>
> Hi!
>
> > > @@ -271,8 +272,8 @@ struct azx_dev {
> > > /* for sanity check of position buffer */
> > > unsigned int period_intr;
> > >
> > > - unsigned int opened: 1;
> > > - unsigned int running: 1;
> > > + unsigned int opened :1;
> > > + unsigned int running :1;
> > > };
> > >
> > > /* CORB/RIRB */
> > > @@ -330,8 +331,8 @@ struct azx {
> > >
> > > /* flags */
> > > int position_fix;
> > > - unsigned int initialized: 1;
> > > - unsigned int single_cmd: 1;
> > > + unsigned int initialized :1;
> > > + unsigned int single_cmd :1;
> > > };
> >
> > Any official standard reference for bit-field expressions?
>
> Well, logically : belongs to the 1, and include/linux understands it
> like that...
OK, "xxx :1;" looks major, too :)
> > > /* driver types */
> > > @@ -642,14 +643,14 @@ static int azx_reset(struct azx *chip)
> > > azx_writeb(chip, GCTL, azx_readb(chip, GCTL) | ICH6_GCTL_RESET);
> > >
> > > count = 50;
> > > - while (! azx_readb(chip, GCTL) && --count)
> > > + while (!azx_readb(chip, GCTL) && --count)
> > > msleep(1);
> >
> > Hm, it looks rather like a personal preference.
> > IMHO, it's harder to read without space...
>
> Well, core parts (sched.c?) use it without the space, and I'd say (!
> expression) is unusual in kernel, but no, could not find it codified.
I don't mind much to change it now, but hopefully people won't be too
strict about this rule...
> > I'll fix the volatile things separately.
The access is really over RAM, not MMIO. So it should be OK to access
in that way. But volatile seems superfluous. I'll get rid of it.
Also, msleep() in the removal should be synchronize_irq().
I'll fix these changes and commit with your space fixes to ALSA tree.
Thanks,
Takashi
On Thu, 31 Aug 2006 15:01:45 +0200 Takashi Iwai wrote:
> At Thu, 31 Aug 2006 14:37:06 +0200,
> Pavel Machek wrote:
> >
> > @@ -271,8 +272,8 @@ struct azx_dev {
> > /* for sanity check of position buffer */
> > unsigned int period_intr;
> >
> > - unsigned int opened: 1;
> > - unsigned int running: 1;
> > + unsigned int opened :1;
> > + unsigned int running :1;
> > };
> >
> > /* CORB/RIRB */
> > @@ -330,8 +331,8 @@ struct azx {
> >
> > /* flags */
> > int position_fix;
> > - unsigned int initialized: 1;
> > - unsigned int single_cmd: 1;
> > + unsigned int initialized :1;
> > + unsigned int single_cmd :1;
> > };
>
> Any official standard reference for bit-field expressions?
Pavel knows how to submit patches to CodingStyle too. :)
> > /* driver types */
> > @@ -642,14 +643,14 @@ static int azx_reset(struct azx *chip)
> > azx_writeb(chip, GCTL, azx_readb(chip, GCTL) | ICH6_GCTL_RESET);
> >
> > count = 50;
> > - while (! azx_readb(chip, GCTL) && --count)
> > + while (!azx_readb(chip, GCTL) && --count)
> > msleep(1);
>
> Hm, it looks rather like a personal preference.
> IMHO, it's harder to read without space...
We have been tending toward not using space in cases like this
(in my unscientific memory-based survey).
So, just this morning I have seen questions and opinions about
the following that could (or could not) use more documentation
or codification and I'm sure that we could easily find more,
but do we want to codify Everything??
1. Kconfig help text should be indented (it's not indented in the
GFS2 patches)
2. if (!condition1) /* no space between ! and condition1 */
3. don't use C99-style // comments
4. unsigned int bitfield :<nr_bits>;
---
~Randy
Hi!
> > > > @@ -271,8 +272,8 @@ struct azx_dev {
> > > > /* for sanity check of position buffer */
> > > > unsigned int period_intr;
> > > >
> > > > - unsigned int opened: 1;
> > > > - unsigned int running: 1;
> > > > + unsigned int opened :1;
> > > > + unsigned int running :1;
> > > > };
> > > >
> > > > /* CORB/RIRB */
> > > > @@ -330,8 +331,8 @@ struct azx {
> > > >
> > > > /* flags */
> > > > int position_fix;
> > > > - unsigned int initialized: 1;
> > > > - unsigned int single_cmd: 1;
> > > > + unsigned int initialized :1;
> > > > + unsigned int single_cmd :1;
> > > > };
> > >
> > > Any official standard reference for bit-field expressions?
> >
> > Well, logically : belongs to the 1, and include/linux understands it
> > like that...
>
> OK, "xxx :1;" looks major, too :)
>
> > > > /* driver types */
> > > > @@ -642,14 +643,14 @@ static int azx_reset(struct azx *chip)
> > > > azx_writeb(chip, GCTL, azx_readb(chip, GCTL) | ICH6_GCTL_RESET);
> > > >
> > > > count = 50;
> > > > - while (! azx_readb(chip, GCTL) && --count)
> > > > + while (!azx_readb(chip, GCTL) && --count)
> > > > msleep(1);
> > >
> > > Hm, it looks rather like a personal preference.
> > > IMHO, it's harder to read without space...
> >
> > Well, core parts (sched.c?) use it without the space, and I'd say (!
> > expression) is unusual in kernel, but no, could not find it codified.
>
> I don't mind much to change it now, but hopefully people won't be too
> strict about this rule...
Thanks.
> > > I'll fix the volatile things separately.
>
> The access is really over RAM, not MMIO. So it should be OK to access
> in that way. But volatile seems superfluous. I'll get rid of it.
Sorry, I understood the code well enough to sense something is wrong,
but not enough to know how to fix it.
> Also, msleep() in the removal should be synchronize_irq().
Good.
> I'll fix these changes and commit with your space fixes to ALSA tree.
Thanks!
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi!
> > Hm, it looks rather like a personal preference.
> > IMHO, it's harder to read without space...
>
> We have been tending toward not using space in cases like this
> (in my unscientific memory-based survey).
>
> So, just this morning I have seen questions and opinions about
> the following that could (or could not) use more documentation
> or codification and I'm sure that we could easily find more,
> but do we want to codify Everything??
>
>
> 1. Kconfig help text should be indented (it's not indented in the
> GFS2 patches)
>
> 2. if (!condition1) /* no space between ! and condition1 */
>
> 3. don't use C99-style // comments
>
> 4. unsigned int bitfield :<nr_bits>;
Looks reasonable to me. Will you do the patch or should I ?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
VGER BF report: U 0.489855
On Sun, 3 Sep 2006 01:15:10 +0200 Pavel Machek wrote:
> Hi!
>
> > > Hm, it looks rather like a personal preference.
> > > IMHO, it's harder to read without space...
> >
> > We have been tending toward not using space in cases like this
> > (in my unscientific memory-based survey).
> >
> > So, just this morning I have seen questions and opinions about
> > the following that could (or could not) use more documentation
> > or codification and I'm sure that we could easily find more,
> > but do we want to codify Everything??
> >
> >
> > 1. Kconfig help text should be indented (it's not indented in the
> > GFS2 patches)
> >
> > 2. if (!condition1) /* no space between ! and condition1 */
> >
> > 3. don't use C99-style // comments
> >
> > 4. unsigned int bitfield :<nr_bits>;
>
> Looks reasonable to me. Will you do the patch or should I ?
Please (you) go ahead with it.
Thanks,
---
~Randy
--
VGER BF report: U 0.5
Hi!
> > > So, just this morning I have seen questions and opinions about
> > > the following that could (or could not) use more documentation
> > > or codification and I'm sure that we could easily find more,
> > > but do we want to codify Everything??
> > >
> > >
> > > 1. Kconfig help text should be indented (it's not indented in the
> > > GFS2 patches)
Chapter 10 speaks about that alread?
Chapter 10: Configuration-files
For configuration options (arch/xxx/Kconfig, and all the Kconfig
files),
somewhat different indentation is used.
Help text is indented with 2 spaces.
if CONFIG_EXPERIMENTAL
tristate CONFIG_BOOM
default n
help
Apply nitroglycerine inside the keyboard (DANGEROUS)
bool CONFIG_CHEER
depends on CONFIG_BOOM
default y
help
Output nice messages when you explode
endif
> > > 2. if (!condition1) /* no space between ! and condition1
*/
> > > 3. don't use C99-style // comments
> > > 4. unsigned int bitfield :<nr_bits>;
Ok.
> > Looks reasonable to me. Will you do the patch or should I ?
>
> Please (you) go ahead with it.
Does it look okay?
Signed-off-by: Pavel Machek <[email protected]>
diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
index 6d2412e..d8e51a6 100644
--- a/Documentation/CodingStyle
+++ b/Documentation/CodingStyle
@@ -46,6 +46,16 @@ used for indentation, and the above exam
Get a decent editor and don't leave whitespace at the end of lines.
+Bitfield variables should be indented like this:
+
+ unsigned int foo :1;
+
+Avoid extra spaces around ! operator, and do not place spaces around (s.
+
+ if (!buffer)
+
+is okay, if ( ! buffer ) is just ugly.
+
Chapter 2: Breaking long lines and strings
@@ -280,7 +290,7 @@ int fun(int a)
int result = 0;
char *buffer = kmalloc(SIZE);
- if (buffer == NULL)
+ if (!buffer)
return -ENOMEM;
if (condition1) {
@@ -316,6 +326,9 @@ When commenting the kernel API functions
See the files Documentation/kernel-doc-nano-HOWTO.txt and scripts/kernel-doc
for details.
+Please do not use C99-style comments, and do not use comments to
+comment out unused code.
+
Chapter 9: You've made a mess of it
That's OK, we all do. You've probably been told by your long-time Unix
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
At Tue, 5 Sep 2006 10:08:14 +0200,
Pavel Machek wrote:
>
> > > Looks reasonable to me. Will you do the patch or should I ?
> >
> > Please (you) go ahead with it.
>
> Does it look okay?
>
> Signed-off-by: Pavel Machek <[email protected]>
>
> diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
> index 6d2412e..d8e51a6 100644
> --- a/Documentation/CodingStyle
> +++ b/Documentation/CodingStyle
> @@ -46,6 +46,16 @@ used for indentation, and the above exam
>
> Get a decent editor and don't leave whitespace at the end of lines.
>
> +Bitfield variables should be indented like this:
> +
> + unsigned int foo :1;
> +
> +Avoid extra spaces around ! operator, and do not place spaces around (s.
> +
> + if (!buffer)
> +
> +is okay, if ( ! buffer ) is just ugly.
> +
I know I belong to an endagered species, but I still find
if (! buffer)
...
isn't so ugly.
Takashi
On Tue, 5 Sep 2006 10:08:14 +0200 Pavel Machek wrote:
> Hi!
>
> > > > So, just this morning I have seen questions and opinions about
> > > > the following that could (or could not) use more documentation
> > > > or codification and I'm sure that we could easily find more,
> > > > but do we want to codify Everything??
> > > >
> > > >
> > > > 1. Kconfig help text should be indented (it's not indented in the
> > > > GFS2 patches)
>
> Chapter 10 speaks about that alread?
OK, good.
I had only looked in Documentation/kbuild/kconfig-language.txt.
> > > > 2. if (!condition1) /* no space between ! and condition1
> */
> > > > 3. don't use C99-style // comments
> > > > 4. unsigned int bitfield :<nr_bits>;
>
> Ok.
>
> > > Looks reasonable to me. Will you do the patch or should I ?
> >
> > Please (you) go ahead with it.
>
> Does it look okay?
>
> Signed-off-by: Pavel Machek <[email protected]>
>
> diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
> index 6d2412e..d8e51a6 100644
> --- a/Documentation/CodingStyle
> +++ b/Documentation/CodingStyle
> @@ -46,6 +46,16 @@ used for indentation, and the above exam
>
> Get a decent editor and don't leave whitespace at the end of lines.
>
> +Bitfield variables should be indented like this:
> +
> + unsigned int foo :1;
> +
> +Avoid extra spaces around ! operator, and do not place spaces around (s.
I would spell out "parentheses".
> + if (!buffer)
> +
> +is okay, if ( ! buffer ) is just ugly.
> +
>
> Chapter 2: Breaking long lines and strings
>
> @@ -280,7 +290,7 @@ int fun(int a)
> int result = 0;
> char *buffer = kmalloc(SIZE);
>
> - if (buffer == NULL)
> + if (!buffer)
> return -ENOMEM;
>
> if (condition1) {
> @@ -316,6 +326,9 @@ When commenting the kernel API functions
> See the files Documentation/kernel-doc-nano-HOWTO.txt and scripts/kernel-doc
> for details.
>
> +Please do not use C99-style comments, and do not use comments to
^
I would insert: "// "
> +comment out unused code.
> +
Is there an acceptable way to leave source code in a file but
render it unused? Like #if 0/#endif or #if BOGUS_SYMBOL/#endif ?
> Chapter 9: You've made a mess of it
>
> That's OK, we all do. You've probably been told by your long-time Unix
---
~Randy
Pavel Machek wrote:
> +Avoid extra spaces around ! operator, and do not place spaces around (s.
How about:
Avoid extra spaces after the ! operator.
Do not place spaces around parentheses.
Because "foo && !bar" is certainly OK.
Or more draconian for the former and less so for the latter rule:
Do not put whitespace between any of the unary operators and
their operand.
It is usually unnecessary to have whitespace around parentheses
as part of expressions, around brackets, or around the operators
. and ->.
Rule 1 certainly applies likewise to ++, --, unary +, unary -, !, ~,
(typecast), unary *, unary &, sizeof.
Rule 2 applies to all of ( ), [ ], ., ->, except where line breaks and
indentation warrant whitespace, or where whitespace helps to read
expressions with more levels of braces. Although the latter should be
avoided anyway.
--
Stefan Richter
-=====-=-==- =--= --=-=
http://arcgraph.de/sr/
Hi,
On Tuesday, 5. September 2006 17:43, Randy.Dunlap wrote:
> Is there an acceptable way to leave source code in a file but
> render it unused? Like #if 0/#endif or #if BOGUS_SYMBOL/#endif ?
They are to reading like gaps in a road.
If you really need to keep unused code for reference
just do it in your LOCAL repository.
There are exceptions where you remove the last user of code
in one patch and a different developer just needs this code
for his new stuff in another patch.
In this case an #if 0/#endif is quite common to reduce merging
conflicts.
Regards
Ingo Oeser
Hi!
> > +comment out unused code.
> > +
>
> Is there an acceptable way to leave source code in a file but
> render it unused? Like #if 0/#endif or #if BOGUS_SYMBOL/#endif ?
I'd say "no way is acceptable, but #if 0/#endif is least evil" :-).
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Wed, 6 Sep 2006, Pavel Machek wrote:
> Hi!
>
>>> +comment out unused code.
>>> +
>>
>> Is there an acceptable way to leave source code in a file but
>> render it unused? Like #if 0/#endif or #if BOGUS_SYMBOL/#endif ?
>
> I'd say "no way is acceptable, but #if 0/#endif is least evil" :-).
I'd say "no way is acceptable, but #if 0/#endif with proper comments is
less evil."
Disabled code will never break if other parts of the code change
without it; rather, it could just become plain wrong. People might either
leave it alone (if they don't know what it is for) or try to change it (if
they think they do).
If you must leave disabled code behind (which in my perfect world would be
'never'), you should at least leave behind a comment explaining what the
code is supposed to do and why it isn't enabled.
If it starts to drift from almost-functional to plain wrong, it becomes an
even worse wart than it originally was.
Thanks,
Chase