User avatar
PeterO
Posts: 4940
Joined: Sun Jul 22, 2012 4:14 pm

Writing clean code isn't hard!

Fri Apr 07, 2017 8:12 am

Just to make a point... My current project....

Code: Select all

petero@HP173 ~/803-GTK3 $ make
Scanning dependencies of target 803
[  8%] Building C object CMakeFiles/803.dir/Emulator.c.o
[ 16%] Building C object CMakeFiles/803.dir/Contactor.c.o
[ 25%] Building C object CMakeFiles/803.dir/Keyboard.c.o
[ 33%] Building C object CMakeFiles/803.dir/Editor.c.o
[ 41%] Building C object CMakeFiles/803.dir/PTS.c.o
[ 50%] Building C object CMakeFiles/803.dir/Reader.c.o
[ 58%] Building C object CMakeFiles/803.dir/SimpleDrawers.c.o
[ 66%] Building C object CMakeFiles/803.dir/LoadInfo.c.o
[ 75%] Building C object CMakeFiles/803.dir/Hand.c.o
[ 83%] Building C object CMakeFiles/803.dir/Cursors.c.o
[ 91%] Building C object CMakeFiles/803.dir/Common.c.o
[100%] Building C object CMakeFiles/803.dir/fsm.c.o
Linking C executable 803
[100%] Built target 803

Code: Select all

petero@HP173 ~/803-GTK3 $ wc *.c 
   198    478   3679 Common.c
   107    190   2768 Contactor.c
   232    630   5106 Cursors.c
  1027   1949  26759 Editor.c
    41     69   1051 Empty.c
    96    213   2236 Emulator.c
    93    153   1569 fsm.c
   639   1273  14534 Hand.c
  1327   2923  34090 Keyboard.c
   441    847   9133 LoadInfo.c
   677   1234  16823 PTS.c
  1860   3932  43989 Reader.c
   588   1104  11817 SimpleDrawers.c
  7326  14995 173554 total
petero@HP173 ~/803-GTK3 $ 
That's Over 7000 lines of code compiled with -Wall -Wextra -Wpedantic with no warnings..... It can be done if you actually take time and care about what you are doing !
PeterO
Discoverer of the PI2 XENON DEATH FLASH!
Interests: C,Python,PIC,Electronics,Ham Radio (G0DZB),1960s British Computers.
"The primary requirement (as we've always seen in your examples) is that the code is readable. " Dougie Lawson

jahboater
Posts: 4596
Joined: Wed Feb 04, 2015 6:38 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 8:29 am

PeterO wrote:That's Over 7000 lines of code compiled with -Wall -Wextra -Wpedantic with no warnings..... It can be done if you actually take time and care about what you are doing !
PeterO
And its best done from the start, then its relatively easy.
I suggest adding -Wconversion to the minimum warning flags above.

I use this in the makefile for a "lint" target.

Code: Select all

WARN = -Wfatal-errors -Wall -Wextra -Wunused -Wconversion -Wundef -Wcast-qual \
       -Wredundant-decls -Wunreachable-code -Wwrite-strings -Warray-bounds    \
       -Wstrict-aliasing=3 -Wstrict-overflow=1 -Wstrict-prototypes -Winline   \
       -Wshadow -Wswitch -Wmissing-include-dirs -Woverlength-strings -Wpacked \
       -Wdisabled-optimization -Wmissing-prototypes -Wformat=2 -Winit-self    \
       -Wmissing-declarations -Wunused-parameter -Wlogical-op -Wuninitialized \
       -Wnested-externs -Wpointer-arith -Wdouble-promotion -Wunused-macros    \
       -Wcast-align -Wunsafe-loop-optimizations 
#  
#  Remove for older compilers
#  
WARN += -Wduplicated-cond -Wnull-dereference -Wshift-overflow=2
Note -O3 may produce more accurate messages (particularly uninitialised variables) due to the greater depth of analysis.

WARN could be used in a simple case thus:

Code: Select all

lint:
  $(CC) prog.c -O3 $(CFLAGS) $(WARN) $(LIBS) -o /dev/null

prog: prog.c
  $(CC) prog.c -Os $(CFLAGS) $(LIBS) -o prog
Last edited by jahboater on Fri Apr 07, 2017 11:43 am, edited 2 times in total.

User avatar
PeterO
Posts: 4940
Joined: Sun Jul 22, 2012 4:14 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 9:43 am

jahboater wrote:
PeterO wrote:That's Over 7000 lines of code compiled with -Wall -Wextra -Wpedantic with no warnings..... It can be done if you actually take time and care about what you are doing !
PeterO
And its best done from the start, then its relatively easy.
I suggest adding -Wconversion to the minimum warning flags above.

I use this in the makefile for a "lint" target.

Code: Select all

WARN = -Wfatal-errors -Wall -Wextra -Wunused -Wconversion -Wundef -Wcast-qual \
       -Wredundant-decls -Wunreachable-code -Wwrite-strings -Warray-bounds    \
       -Wstrict-aliasing=3 -Wstrict-overflow=1 -Wstrict-prototypes -Winline   \
       -Wshadow -Wswitch -Wmissing-include-dirs -Woverlength-strings -Wpacked \
       -Wdisabled-optimization -Wmissing-prototypes -Wformat=2 -Winit-self    \
       -Wmissing-declarations -Wunused-parameter -Wlogical-op -Wuninitialized \
       -Wnested-externs -Wpointer-arith -Wdouble-promotion -Wunused-macros    \
       -Wunsafe-loop-optimizations 
#  
#  Remove for older compilers
#  
WARN += -Wduplicated-cond -Wnull-dereference -Wshift-overflow=2
Note -O3 may produce more accurate messages (particularly uninitialised variables) due to the greater depth of analysis.
I find -Wconversion produces "noise" from my code. I'm quite happy to let the compiler silently deal with some implicit type conversions in my code. I guess I'm a bit "old school" in this respect.
For example, I'm quite happy to assign a float to an int when the float is the x coordinate from a mouse event. Next time I start a new file I'll add it :-)

I'm not going to show you the compiler output with -Wconversion enabled because one of the things I'm doing in that project is converting one of the main structure's members from signed to unsigned ints and at the moment I've only tackled about half of the uses.

PeterO
Discoverer of the PI2 XENON DEATH FLASH!
Interests: C,Python,PIC,Electronics,Ham Radio (G0DZB),1960s British Computers.
"The primary requirement (as we've always seen in your examples) is that the code is readable. " Dougie Lawson

User avatar
DougieLawson
Posts: 35784
Joined: Sun Jun 16, 2013 11:19 pm
Location: Basingstoke, UK
Contact: Website Twitter

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 10:09 am

jahboater wrote: I use this in the makefile for a "lint" target.

Code: Select all

-Wfatal-errors -Wall -Wextra -Wunused -Wconversion -Wundef -Wcast-qual \
       -Wredundant-decls -Wunreachable-code -Wwrite-strings -Warray-bounds    \
       -Wstrict-aliasing=3 -Wstrict-overflow=1 -Wstrict-prototypes -Winline   \
       -Wshadow -Wswitch -Wmissing-include-dirs -Woverlength-strings -Wpacked \
       -Wdisabled-optimization -Wmissing-prototypes -Wformat=2 -Winit-self    \
       -Wmissing-declarations -Wunused-parameter -Wlogical-op -Wuninitialized \
       -Wnested-externs -Wpointer-arith -Wdouble-promotion -Wunused-macros    \
       -Wunsafe-loop-optimizations 
It would be useful is any folks who are providing code for beginners on this forum would compile their stuff with at least some of those flags. There's too much with invalid main() functions (returns void, does return int or simply uses return;) which isn't good for the folks who are new to C programming. If it's some junk example for something I'm trying for myself (before including it in a larger project) it probably doesn't matter. As soon as you publish it for other folks it should cleanly pass most of those compiler tests.
Note: Having anything humorous in your signature is completely banned on this forum. Wear a tin-foil hat and you'll get a ban.

Any DMs sent on Twitter will be answered next month.

This is a doctor free zone.

jahboater
Posts: 4596
Joined: Wed Feb 04, 2015 6:38 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 10:41 am

PeterO wrote: I find -Wconversion produces "noise" from my code. I'm quite happy to let the compiler silently deal with some implicit type conversions in my code.
Fair enough.

For interest ...
It does "Warn for implicit conversions that may alter a value" (man gcc), benign conversions are silent.
int32_t n;
double x;
x = n;
will not raise a warning because the mantissa is 53 bits, bigger than an int.
but
int64_t n;
double x;
x = n;
will fail because something must be lost when a 64 bit integer is squeezed into a 53 bit mantissa.
More interestingly,
unsigned int a;
long b;
b = a;
will fail on the Pi in 32-bit mode because the sign could change, but succeed and not raise a warning on a 64-bit LP64 machine. You get different results on different platforms.

jahboater
Posts: 4596
Joined: Wed Feb 04, 2015 6:38 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 11:21 am

-Wall/-Wextra surprisingly doesn't cover all that much, I don't know why more options are not included.
For example, daft code like:

static const char mystr[] = "asasdddad";
char *ptr = (char*)mystr;
*ptr = 'x';

will likely crash the program and I would always want to know about it (-Wcast-qual is needed).

User avatar
Paeryn
Posts: 2633
Joined: Wed Nov 23, 2011 1:10 am
Location: Sheffield, England

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 11:55 am

jahboater wrote:-Wall/-Wextra surprisingly doesn't cover all that much, I don't know why more options are not included.
For example, daft code like:

static const char mystr[] = "asasdddad";
char *ptr = (char*)mystr;
*ptr = 'x';

will likely crash the program and I would always want to know about it (-Wcast-qual is needed).
Probably down to with C casting you are taking responsibility for what you are doing when you cast and there is only one way of explicit casting. Having the warning in the default sets could cause a lot of messages in lagacy code that uses it safely (i.e. doesn't attempt to write into the const).
She who travels light — forgot something.

User avatar
PeterO
Posts: 4940
Joined: Sun Jul 22, 2012 4:14 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 11:58 am

I've added your "long list" of warning switches to my CMakeList.txt file, and now I feel like a noob C programmer again :roll:
But I'm biting the bullet and will try to get back to a clean compile, but it might take me quite a while 8-)

PeterO
Discoverer of the PI2 XENON DEATH FLASH!
Interests: C,Python,PIC,Electronics,Ham Radio (G0DZB),1960s British Computers.
"The primary requirement (as we've always seen in your examples) is that the code is readable. " Dougie Lawson

User avatar
PeterO
Posts: 4940
Joined: Sun Jul 22, 2012 4:14 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 12:19 pm

I can't live with some of those warnings !! My code is careful to make sure functions come before their first use thus avoiding lots of pointless prototypes. I've had to disable the warnings that complain if functions are not prototyped even when they are defined before they are used.

PeterO
Discoverer of the PI2 XENON DEATH FLASH!
Interests: C,Python,PIC,Electronics,Ham Radio (G0DZB),1960s British Computers.
"The primary requirement (as we've always seen in your examples) is that the code is readable. " Dougie Lawson

jahboater
Posts: 4596
Joined: Wed Feb 04, 2015 6:38 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 12:36 pm

PeterO wrote:I can't live with some of those warnings !! My code is careful to make sure functions come before their first use thus avoiding lots of pointless prototypes. I've had to disable the warnings that complain if functions are not prototyped even when they are defined before they are used.
Are you sure??? My code is the same as yours, I define functions before using them and don't bother with the prototypes where possible - and it does compile cleanly.

If you do get bored, I noticed that -fstrict-aliasing and -fstrict-overflow must be switched on (mine is in CFLAGS) before -Wstrict-aliasing and -Wstrict-overflow become active.

Yes, some warnings can be safely elided. For example -Wcast-align throws up a couple of annoying warnings where I have carefully and deliberately ensured the target alignment is correct, but the compiler doesn't know it.

User avatar
PeterO
Posts: 4940
Joined: Sun Jul 22, 2012 4:14 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 1:09 pm

jahboater wrote:
PeterO wrote:I can't live with some of those warnings !! My code is careful to make sure functions come before their first use thus avoiding lots of pointless prototypes. I've had to disable the warnings that complain if functions are not prototyped even when they are defined before they are used.
Are you sure???
Yes. -Wmissing-prototypes and -Wmissing-declarations both needed to be removed to get ride of this warning:

Code: Select all

/home/petero/803-GTK3/Editor.c:77:10: warning: no previous prototype for ‘transmitterIsEmpty’ [-Wmissing-prototypes]
 gboolean transmitterIsEmpty(void)
Adding a totally pointless prototype just before the actual function stops the warning message!
From gcc manual:
gcc manual wrote: -Wmissing-prototypes (C and Objective-C only)

Warn if a global function is defined without a previous prototype declaration. This warning is issued even if the definition itself provides a prototype. Use this option to detect global functions that do not have a matching prototype declaration in a header file. This option is not valid for C++ because all function declarations provide prototypes and a non-matching declaration declares an overload rather than conflict with an earlier declaration. Use -Wmissing-declarations to detect missing declarations in C++.

-Wmissing-declarations

Warn if a global function is defined without a previous declaration. Do so even if the definition itself provides a prototype. Use this option to detect global functions that are not declared in header files. In C, no warnings are issued for functions with previous non-prototype declarations; use -Wmissing-prototypes to detect missing prototypes. In C++, no warnings are issued for function templates, or for inline functions, or for functions in anonymous namespaces.
I'm definitely not going to use them as they seem to warn against what is perfectly valid C ! I don't see how your code isn't falling foul of them like mine does.
PeterO
Last edited by PeterO on Fri Apr 07, 2017 1:32 pm, edited 1 time in total.
Discoverer of the PI2 XENON DEATH FLASH!
Interests: C,Python,PIC,Electronics,Ham Radio (G0DZB),1960s British Computers.
"The primary requirement (as we've always seen in your examples) is that the code is readable. " Dougie Lawson

jahboater
Posts: 4596
Joined: Wed Feb 04, 2015 6:38 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 1:31 pm

Interesting!
What version of the compiler are you using?
Messages from gcc have improved quite a bit recently.

jahboater
Posts: 4596
Joined: Wed Feb 04, 2015 6:38 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 1:41 pm

PeterO wrote: I'm definitely not going to use them as they seem to warn against what is perfectly valid C ! I don't see how your code isn't falling foul of them like mine does.
PeterO
Neither do I (understand). I tried and its clean with gcc 4.9 and gcc 6.3. And its a larger program (over 15,000 lines).
I have also

Code: Select all

#  
#  GCC options
#
OPT += -fno-math-errno -fno-associative-math -fno-unsafe-math-optimizations \
       -fno-reciprocal-math -ffinite-math-only

OPT += -finline-functions -finline-small-functions -findirect-inlining \
       -fpartial-inlining -finline-functions-called-once -fearly-inlining

OPT += -funsigned-char -fwhole-program -fno-exceptions -fomit-frame-pointer       \
       -fstrict-aliasing -fstrict-overflow -ftabstop=4 -fexpensive-optimizations  \
       -fno-stack-protector -fno-unwind-tables -fno-asynchronous-unwind-tables    \
       -funsafe-loop-optimizations -foptimize-strlen -fmerge-all-constants -free  \
       -faggressive-loop-optimizations -fipa-pta -freg-struct-return
I wonder if -fwhole-program has anything to do with it.

User avatar
PeterO
Posts: 4940
Joined: Sun Jul 22, 2012 4:14 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 1:55 pm

jahboater wrote:Interesting!
What version of the compiler are you using?
Messages from gcc have improved quite a bit recently.
4.6.3 on Linux Mint 17.3
4.9.2 on Pi
5.4.0 on Linux Mint 18

All seem to produce the same warnings in this situation.

PeterO

PS: -fwhole-program just breaks the link stage in a strange way !
Discoverer of the PI2 XENON DEATH FLASH!
Interests: C,Python,PIC,Electronics,Ham Radio (G0DZB),1960s British Computers.
"The primary requirement (as we've always seen in your examples) is that the code is readable. " Dougie Lawson

jahboater
Posts: 4596
Joined: Wed Feb 04, 2015 6:38 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 2:29 pm

Well, at least you have done the investigation, proved its a false positive and can now safely remove those flags!

I guess its difficult, and perhaps too much to ask, but could you reproduce the problem in a small, self contained piece of code? If so, and it fails with gcc 6.3, I should perhaps submit it as a low priority bug to the gcc folks.

User avatar
PeterO
Posts: 4940
Joined: Sun Jul 22, 2012 4:14 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 2:40 pm

jahboater wrote:Well, at least you have done the investigation, proved its a false positive and can now safely remove those flags!

I guess its difficult, and perhaps too much to ask, but could you reproduce the problem in a small, self contained piece of code? If so, and it fails with gcc 6.3, I should perhaps submit it as a low priority bug to the gcc folks.
I'll have a go (but it might be tomorrow evening before I get time to sort it out), As far as I can see the warning switches are doing what the manual says they should (see high lighted text in manual quote above). There must be another option in your set up that has some influence.

PeterO
Discoverer of the PI2 XENON DEATH FLASH!
Interests: C,Python,PIC,Electronics,Ham Radio (G0DZB),1960s British Computers.
"The primary requirement (as we've always seen in your examples) is that the code is readable. " Dougie Lawson

jahboater
Posts: 4596
Joined: Wed Feb 04, 2015 6:38 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 2:45 pm

PeterO wrote: There must be another option in your set up that has some influence.
yes may well be.
Only bother with a test case if you are convinced its a bug, otherwise forget about it!

User avatar
PeterO
Posts: 4940
Joined: Sun Jul 22, 2012 4:14 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 2:52 pm

I'm getting frustrated by lots of warnings about my mixing signed and unsigned values :shock:

For example libraries like Cairo and Gtk return signed values for the size of things like windows and wigets. But then things like memset want size_t for the size of things. I guess the trick is to minimise the number of casts
needed to convince the compiler that you do actually know what you are doing :lol:

PeterO
Discoverer of the PI2 XENON DEATH FLASH!
Interests: C,Python,PIC,Electronics,Ham Radio (G0DZB),1960s British Computers.
"The primary requirement (as we've always seen in your examples) is that the code is readable. " Dougie Lawson

User avatar
joan
Posts: 14175
Joined: Thu Jul 05, 2012 5:09 pm
Location: UK

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 3:02 pm

I'll give this a miss and stick with -Wall. Reminds me of Ada programmers who assumed because a program clean compiled it must be correct.

User avatar
PeterO
Posts: 4940
Joined: Sun Jul 22, 2012 4:14 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 3:18 pm

joan wrote:I'll give this a miss and stick with -Wall. Reminds me of Ada programmers who assumed because a program clean compiled it must be correct.
I tend to agree with sticking with -Wall and -Wextra, but then I also know I'm a bit behind the times when it comes to some aspects of modern C code. I've never taken the time to
update myself so this seems like an ideal opportunity to take a step back and spend some time catching up on a few things.

For example I'm sure that somewhere in 7000 lines of code there should be at least a few places where I should use "const" :lol: :roll: :shock:
PeterO
Discoverer of the PI2 XENON DEATH FLASH!
Interests: C,Python,PIC,Electronics,Ham Radio (G0DZB),1960s British Computers.
"The primary requirement (as we've always seen in your examples) is that the code is readable. " Dougie Lawson

jahboater
Posts: 4596
Joined: Wed Feb 04, 2015 6:38 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 3:47 pm

and who said "Writing clean code isn't hard" ;)

Declare everything const that doesn't change - then you will get lots more warnings!

.... but faster and most importantly, safer, code.

User avatar
PeterO
Posts: 4940
Joined: Sun Jul 22, 2012 4:14 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 4:49 pm

jahboater wrote:and who said "Writing clean code isn't hard" ;)
I'm well aware of the irony ! But there is "clean" and then there's "all shiny and clean". People that post code that isn't even clean with just "-Wall -Wextra" were the real target of this thread :D

PeterO
Discoverer of the PI2 XENON DEATH FLASH!
Interests: C,Python,PIC,Electronics,Ham Radio (G0DZB),1960s British Computers.
"The primary requirement (as we've always seen in your examples) is that the code is readable. " Dougie Lawson

jahboater
Posts: 4596
Joined: Wed Feb 04, 2015 6:38 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 6:26 pm

PeterO wrote:People that post code that isn't even clean with just "-Wall -Wextra" were the real target of this thread
Recent threads have code posted that doesn't even compile!

User avatar
PeterO
Posts: 4940
Joined: Sun Jul 22, 2012 4:14 pm

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 10:35 pm

jahboater wrote:Recent threads have code posted that doesn't even compile!
Indeed, which wastes everyone's time !
PeterO
Discoverer of the PI2 XENON DEATH FLASH!
Interests: C,Python,PIC,Electronics,Ham Radio (G0DZB),1960s British Computers.
"The primary requirement (as we've always seen in your examples) is that the code is readable. " Dougie Lawson

User avatar
Paeryn
Posts: 2633
Joined: Wed Nov 23, 2011 1:10 am
Location: Sheffield, England

Re: Writing clean code isn't hard!

Fri Apr 07, 2017 11:42 pm

The -Wmissing-prototypes and -Wmissing-declarations warnings aren't meant as warnings against bad code, they are more for checking that external functions have declarations in header files. If you decided to rename a function in the source code of a library but forgot to rename it in the header file then the compiler would flag it up as the function would no longer have a matching declaration in the header. Just compiling the library wouldn't necessarily pick up on that if the function is never called from within the library itself.

missing-prototypes is for catching when you declare a function without giving a prototype. This will catch cases where you don't provide the function parameters in the header (which missing-declarations won't).
She who travels light — forgot something.

Return to “C/C++”