`three = 1` in the Linux sourcecode (2014)

[CWuestefeld]

Many years ago in an age of klocs and flowcharts, at a large three-letter computer company, there were iron-clad coding rules that must be obeyed, no questions allowed. In general you could see their reasoning, but such bureaucratic reasoning doesn't pay off.

One example of this was that all numeric values used in a program must be factored out as symbolic constants. The reason for doing this is obvious, but it failed to account for the fact that some numbers have intrinsic meaning that should allow their use directly. But to be compliant with the standard, our C code had this boilerplate up near the top:

  #define zero 0
  #define one  1

This, of course, only served to make every page of code harder to read. And it didn't really even solve the problem it was meant to. We once found in some source code:

  #define thirteen 13

which of course did nothing to show us why that particular value was relevant.

[hyperman1]

An application I touched a few weeks ago has a DB column named 'type' with values 1,2,3. So on to the source code we go:

  enum RecordType {TypeOne(1),TypeTwo(2),TypeThree(3); RecordType(int dbValue) ...}

As it happens, I know an end user of this particular beast, so I show her some record IDs of each type and ask in what way they differ. She looks a few seconds, then says: 'This is clearly a type one record, the next is a type two, and here you have a type three'. After a little back an forth, it turns out even our internal course for new personnel talks about 'records' with different rules to follow for 'type one/two/three'. The end users have no other word to describe these entities than 'records' each with its own numbered type, and think about it as completely natural. The application is based on an older COBOL application, and the terminology just stuck around.

Clearly, the application is well-written: The enum uses the terminology used by the business. Another of my flabbers just got ghasted.

[yiyus]

I would do this:

    #define I     1
    #define II    2
    #define III   3
    ...
    #define XIII 13

There is no zero in roman numerals, but you can use NULL for extra fun! (yes, I am a bad person)

[hannasanarion]

Why is the reason for doing this obvious? I can't see any benefit.

I get why you would prefer to name constants for their use, like `numIterations=3` or whatever, but renaming every integer seems senseless.

[l0b0]

  #define zero 0

is just bad programming, barring some philosophical code which cares about "zero-ness". Names need to reflect what they represent, not their precise values. Are we looking up the first index? Then how about

  #define first_index 0

? Are we summing up values which are coerced to zero when empty? Then how about

  #define empty_value_integer 0

or

  #define additive_identity 0

?

Yes, that means there might be more than one variable with a value of zero, and that's completely fine since we now have several megabytes available for both source code and for compilation.

To be clear, I'm not endorsing the rule, just saying that there are more productive ways of dealing with it.

[waynesonfire]

'zero' and '0' are the same label, for the digit 0. There is no advantage to re-defining it, in fact, it does the opposite. It's a good practice to define labels for magic numbers to promote more maintainable code through the DRY / single source of truth principle, document the magic number through the label name, and improve code archeology by enabling searching by label.

These principles hold when the label given a clear and concise name--which is an art.

[wycy]

We have something similar in my my organization's Fortran code. We have constants KI1, KI2, KI3, ... etc corresponding to (k)onstant integers 1, 2, 3, ... etc. Someone got the idea that hard-coded numbers were bad. All it does is make code harder to read to anyone unfamiliar with these conventions.

[cratermoon]

Did you have an automatic static code analysis tool to flag non-compliance? This is the sort of thing I see and use to warn people away from blindly spending money on this sort of thing, installing the tool, and calling it good.

[post-factum]

  705 /*
  706  * Iterate through the groups which hold BACKUP superblock/GDT copies in an
  707  * ext4 filesystem.  The counters should be initialized to 1, 5, and 7 before
  708  * calling this for the first time.  In a sparse filesystem it will be the
  709  * sequence of powers of 3, 5, and 7: 1, 3, 5, 7, 9, 25, 27, 49, 81, ...
  710  * For a non-sparse filesystem it will be every group: 1, 2, 3, 4, ...
  711  */
  712 static unsigned ext4_list_backups(struct super_block *sb, unsigned *three,
  713                   unsigned *five, unsigned *seven)

[hinoki]

Yeah, maybe calling them pow3, pow5, pow7 would be a bit clearer? But a 10s search for a comment cleared it up, so I don’t really see the problem...

[lifthrasiir]

7 years ago: https://news.ycombinator.com/item?id=7296497

[asplake]

keypusher on Feb 25, 2014:

If only there was a system for fixing the code l yourself instead of having to post about it to a widely read tech site.

[SavantIdiot]

Magic numbers are a horrible pattern (and I don't mean elf/obj IDs).

*ANY* time you use a non-obvious constant (like start of a loop that isn't 0), good practice to comment immediately at the source, or pull it into a well-named variable/macro.

Don't agree? Spend 35 years programming and you'll see this pop up again and again and again and again... SO much time wasted figuring out magic #s.

[olarva]

Explanatory comment:

https://github.com/torvalds/linux/blob/d158fc7f36a25e19791d2...

[brainfog]

Yikes, that looks error prone.

[GuB-42]

Another fun one is ONE = 256 (or another power of two).

Intended for fixed point arithmetic where 1 actually means 1/256.

[ant6n]

This is nicer example, since the posted one should really have different variable names (e.g. pow3). On the other hand, if this declaration of ONE is global (not contained within some context like a type), then perhaps it should be something like ONE_F8.

[lgregg]

Looks like it's still there: https://github.com/torvalds/linux/blob/master/fs/ext4/resize...

[dyingkneepad]

I don't know about the ext4 code specifically, but in hardware programming it is common to have stuff like "3 means 5". If for whatever reason a field of a register may only accept values that actually mean 1, 5 and 7 (to program whatever bits of the hardware, like clock divisors or whatnot), the hw register will probably only contain 2 bits, and be encoded in such a way where if you write 00b you mean 1d, if you write 01b you mean 5d and if you write 10d you mean 7. And 11b is reserved. This is a common thing, it's encoding to pack a sparse set of possible values into a few bits as you can.

But then, you should probably do something such as "#define CLOCK_DIVISOR_3 0x1" instead of simply "int three = 1".

[coding123]

It was done that way on purpose to cause hundreds of people a year to freak out and discuss it ad nauseum. But really a social experiment designed to show everyone is all talk and no follow through... That being that no one actually tried to change open source but just talk about it.

[ddtaylor]

that's interesting, is there a source?

[fargle]

What wrong with:

  _3 = 1;

It's shorter. I know, _ is probably reserved, so maybe:

  x3 = 1; // x is not multiply

There. I fixed it for you.

[sabhiram]

It's actually worse than that, given that all 3, 5 and 7 have 1 as their power-0, and the rest seem to encode powers of the former. Too much WTF and not enough comments.

[trhway]

to me it is a great message that if you have attitude that your predecessors were stupid morons and you don't understand what happens here nor willing to bother to dig enough deep to understand it, you shouldn't be doing changes here. Attempt to correct it is a kind of warning signal for a PR containing the correction.

[karmakaze]

That was 2014, what does it say now?

[aaossa]

No changes: https://github.com/torvalds/linux/blob/master/fs/ext4/resize...

[known]

That's smart; Imagine user setting vm.swappiness = 100 in /etc/sysctl.conf

[amelius]

For small values of three.

[tln]

I wonder why a struct to hold the cursor state wasn't used.

[dis-sys]

surely we need another linter that can catch such issues

[Bost]

It's in the repository of a certain "torvalds". Better check twice next time what this guy's doing :)

[rkangel]

I once spent two days debugging confusing behaviour in a serial command interface before discovering the line:

    #define CR_LF "\n"