
Recommendations about coding in C++ - c-plus-plus
https://software.intel.com/en-us/articles/the-ultimate-question-of-programming-refactoring-and-everything
======
YSFEJ4SWJUVU6
A quick question item 13 (table-style formatting): the author proposes
something like this:

    
    
      static bool IsInterestingError(int errno) { ... }
      ...
      if (!IsInterestingError(errno)) { ... }
    

Can you do that in a single file – assuming the latter errno is the one
defined in errno.h (or cerrno). I was of the impression that errno may also be
a macro, so could you say it's a good recommendation to have it as an
argument's name – or in general even to reuse the name anywhere?

I agree that the option (of passing the helper function no arguments at all)
is not perfect either, but... (I also don't like that you need to call your
stored errno's errnums or the like, but that's what we have.)

~~~
acqq
If errno can be a macro, as at least one man page claims, the argument in the
function could expand and even not compile or produce something unwanted. I'd
prefer:

    
    
       static bool IsInterestingError(int e) { ... }
    

[http://man7.org/linux/man-pages/man3/errno.3.html](http://man7.org/linux/man-
pages/man3/errno.3.html)

~~~
wahern
errno is a macro on most (maybe every) platform supporting pthreads.

Furthermore, C11 literally defines errno as a macro in 7.5p1:

    
    
      The macros are
    
             EDOM
             EILSEQ
             ERANGE
    
      which expand to integer constant expressions with type int, distinct positive values, and
      which are suitable for use in #if preprocessing directives; and
    
             errno
    
      which expands to a modifiable lvalue that has type int and thread local storage
      duration, the value of which is set to a positive error number by several library functions.
      If a macro definition is suppressed in order to access an actual object, or a program
      defines an identifier with the name errno, the behavior is undefined.
    

Notably, C11 introduced an optional threading API, effectively derived from
the common functionality between Window's threading API and POSIX pthreads.
While I can imagine ways to have a unique errno symbol that doesn't rely on
macros, the obvious solution does and there's little reason not to use it.

While C11 is most definitely not the same thing as C++11, I think, similar to
atomics, the committees cooperated on the threading API. I don't have a recent
C++ standard handy at the moment, but I wouldn't be surprised if C++11 at
least allowed defining errno as a macro.

------
amq
> Never dereference null pointers

Something's wrong with us if this has to be mentioned in a list like this.

~~~
msbarnett
You'd think. But there was a long argument here[1] not even a week ago with
multiple people passionately asserting that they were entitled to be able to
rely on completely predictable and non-fatal semantics when derefrencing a
null pointer.

[1]:
[https://news.ycombinator.com/item?id=13640505](https://news.ycombinator.com/item?id=13640505)

~~~
catnaroek
The problem with C and C++ isn't so much that undefined behavior exists, but
the fact that the rules that determine when a program has defined behavior
aren't formalized rigorously enough. The C and C++ specifications are written
in natural language, which always leaves room for ambiguity, no matter how
pedantically one tries to write.

If you want to convince a single other person that a C program you have
written has no undefined behavior, then you and that person have to hopefully
interpret English the same way. It would be completely hopeless to try to
convince a hundred people that a nontrivial C program has no undefined
behavior. And successful programs typically have way more than a hundred
users.

------
asitdhal
Is it an advertisement of PVS-Studio?

~~~
jasode
Yes, it _is_ an ad for PVS-Studio but I don't think that should tarnish the
_content_ of code discussion in the article.

Although I'm extreme anti-advertisement (with dedicated firewall to block web
ads), I find this article having 3 qualities that's valuable:

1) instead of showing contrived bad code nobody writes, it shows _real bugs_
from _real projects_ such as MySQL, PostgreSQL, ReactOS, Apache server,
Audacity, LibreOffice, Notepad++, CoreCLR, etc

2) full explanations of the bug and further advice for correcting the code

3) learning is free: you don't have to buy PVS-Studio to apply the solutions
learned from the article or gain the ability to spot similar bugs later

That all adds up to a quality article which is better than the old PC-Lint
ads[1] from Gimpel Software. For those too young to be aware of them, in the
1990s, GS ran ads in a magazine called C/C++ Users Journal. For the others
that enjoyed the "puzzles" in those ads, Andrey Karpov's article is a fine
continuation of that.

[1] example:
[http://www.polyweb.com/blog/index.php/archives/9](http://www.polyweb.com/blog/index.php/archives/9)

~~~
asitdhal
Thanks for replying...I didn't realize the real life example.

------
leovonl
Funny how the "proper" first example still has a lot of things to be
addressed, like the fact that it has a magic number (7) coming from nowhere,
which is duplicated a few times as well.

It would be way less error-prone to calculate the number of elements using
sizeof, for instance. This would also make the function independent of the
type (a template could be used), thus reusable in another contexts.

------
ixtli
> As a rule, simple code is usually correct code.

This is a strong statement I'm a bit wary of. I get the intent in context but
it seems overly general.

~~~
vietjtnguyen
He does say "usually" so it doesn't seem like that strong of a statement.

------
chris_wot
Item 5 is interesting, but I think it might be worthwhile to include the
entire function here (including the comments):

    
    
      BOOL WINAPI DllMain( HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved )
      {
          (void)hinstDLL; /* avoid warning */
          (void)lpvReserved; /* avoid warning */
          switch (fdwReason)
          {
              case DLL_PROCESS_ATTACH:
              {
                  TCHAR   szBuffer[64];
      
                  // This code will attach the process to its parent process
                  // if the parent process had set the environment variable.
                  // The corresponding code (setting the environment variable)
                  // is desktop/win32/source/officeloader.cxx
      
                  DWORD   dwResult = GetEnvironmentVariable( "ATTACHED_PARENT_PROCESSID", szBuffer, sizeof(szBuffer) );
      
                  if ( dwResult && dwResult < sizeof(szBuffer) )
                  {
                      DWORD   dwThreadId = 0;
      
                      DWORD_PTR   dwParentProcessId = (DWORD_PTR)atol( szBuffer );
      
                      if ( dwParentProcessId && GetParentProcessId() == dwParentProcessId )
                      {
                          // No error check, it works or it does not
                          // Thread should only be started for headless mode, see desktop/win32/source/officeloader.cxx
                          CreateThread( NULL, 0, ParentMonitorThreadProc, (LPVOID)dwParentProcessId, 0, &dwThreadId );
                          // Note: calling CreateThread in DllMain is discouraged
                          // but this is only done in the headless mode and in
                          // that case no other threads should be running at startup
                          // when sal3.dll is loaded; also there is no
                          // synchronization with the spawned thread, so there
                          // does not appear to be a real risk of deadlock here
                      }
                  }
      
                  return TRUE;
              }
      
              case DLL_THREAD_ATTACH:
                  break;
      
              case DLL_THREAD_DETACH:
                  osl_callThreadKeyCallbackOnThreadDetach( );
                  break;
          }
      
          return TRUE;
      }
    

You can find there here:
[http://opengrok.libreoffice.org/xref/core/sal/osl/w32/dllent...](http://opengrok.libreoffice.org/xref/core/sal/osl/w32/dllentry.c#DllMain)

FWIW, he says to use static analyser tools to analyze code. We do that
already, and extensively. Seems a bit provocative...

~~~
AndreyKarpov
I do not remember this comment. Most likely he appeared after the article:
[http://www.viva64.com/en/b/0308/](http://www.viva64.com/en/b/0308/)

~~~
chris_wot
That's quite possible. It might be worthwhile pointing to the revision and
file in git :-)

Appreciate the work you have done!

~~~
AndreyKarpov
sal: add comment re: V718 'CreateThread' should not be called from 'DllMain'

[http://opengrok.libreoffice.org/diff/core/sal/osl/w32/dllent...](http://opengrok.libreoffice.org/diff/core/sal/osl/w32/dllentry.c?r2=%2Fcore%2Fsal%2Fosl%2Fw32%2Fdllentry.c%40279b6e4b75e74c743ca018435c1d7644c71b9e0c&r1=%2Fcore%2Fsal%2Fosl%2Fw32%2Fdllentry.c%40da40cac540e7d735edbe9069b3c8ec6af4530208)

------
monomaniar
Fist I have a low IQ. Then flowing is my comment:

This is the 42 reasons why I give up C++.

~~~
Too
As much as I also hate many parts of c++, to be fair, half of the examples
apply to any programming language.

------
infinisil
The title is very misleading. I was expecting a discussion about the future of
programming, why there will probably never be a "best" language or why we're
not all using functional languages yet. Was disappointed after reading the
first sentence:

> Yes, you've guessed correctly - the answer is "42". In this article you will
> find 42 recommendations about coding in C++ that can help a programmer avoid
> a lot of errors, save time and effort.

The article states that these tips are 'usually universal', but I hardly found
that to be true, only having counted about 10 points not being about C++.

The title should probably be something like the obvious "42 tips for C++
programmers" as the current one really doesn't justify the content.

~~~
dang
OK, we changed the title to be less misleading, as the HN guidelines request
([https://news.ycombinator.com/newsguidelines.html](https://news.ycombinator.com/newsguidelines.html)).

------
kabes
Meh, these recommendations basically boil down to: "pay attention"

~~~
vmarsy
Indeed, however every programmer no matter how good will make mistakes
sometimes. It can be in 1%, 0.1%, 0.01% of LoC. Loosing attention can be so
easy, it can be a co-worker walking by, an important email/meeting happening
while you were focused on a 'copy-pasting' step.

Having a tool that is trained to spot this kind of human errors is good. It
can be useful in any language, not only C++. Avoiding them in the first place
and giving you advices on how to not do it again would be a nice thing
compiler could provide. i.e:

    
    
        a[0] = b[0];
        a[1] = b[1];
        a[2] = b[0];
                 ^ warning: Are you sure you want to assign b[0] to a[2]. If not, consider rewriting your code with a for loop
        a[3] = b[3];

~~~
acqq
The author of the article actually develops the tool that can help the
programmers detect such errors. The compilers don't report such kind of errors
because it was traditionally to expensive to detect them. The special tool,
like the one mentioned in the article, doesn't have to produce the optimized
code and doesn't have to be run as frequently as on each compilation but it
can allow the programmer to detect potential problems.

It's surely worth using it.

------
tempodox
Seeing that list of error patterns made me realize that letting mediocre
programmers use something like C / C++ really boils down to Luck-Driven
Programming. I hope that in the not too distant future something along the
lines of Rust or Swift can help avoid at least some of those traps.

~~~
chris_wot
There are quite a few errors in this article that could occur in Rust and
Swift as well. Did you actually read this article?

~~~
tempodox
There is no silver bullet. And no language can stop you from making mistakes
(assuming “at least some” means the same as “all” is one example). But some
languages help you avoid certain classes of mistakes with automated checks.
Like Rust.

And even if you added those features to C++, you couldn't guarantee a codebase
that only uses the secure subset. Nobody has the time to rewrite old code. And
making sure that current code uses only the secure subset would require code
reviews. Not everyone does that.

~~~
chris_wot
If nobody has time to rewrite old C++ code then it's not going to be rewritten
in rust

~~~
tempodox
That would seem to be the case, naturally. Why do you mention it?

------
quizotic
If you're an experienced C++ coder, I'd skip this article.

While it's well structured, with real-world examples, it's very basic. I'm not
an excellent C++ coder by any stretch of the imagination. I make lots of
mistakes. But not these. The article is fairly thorough and long, so if you're
experienced you'll invest non-trivial time for little or no reward.

~~~
dleslie
The mistakes mentioned include those made in mature projects maintained by
good people.

We all make mistakes; and it's oftenworthwhile to review what we take for
granted that we know.

------
gravypod
Every one of these points is just "use a static analyzer" and while I
appreciate that I definetly don't appreciate them pushing Microsoft's static
analysis tooling rather then Free tooling.

I'd honestly welcome a post about different static analysis tooling that's
libre and open source. Their pros and cons as well as how to deploy them would
be good too.

~~~
pjmlp
How do those people dare to charge money and sell proprietary products!

Also this is about PVS-Studio, nothing to do with Microsoft, they don't own
the _Studio_ word.

~~~
gravypod
I have no problem with paying for a program with my money. I do have a problem
paying for a program with my freedom.

~~~
cjhanks
PVS is a product that performs a service. They have made major efforts to port
this product to Linux; they have multiple articles scanning Linux FSF products
and the Linux Kernel.

People deserve to be paid for their work if it's useful.

------
jjallen
Wow, the first example is simply a typo as a result of copy and pasting the
same conditional block repeatedly. You would learn this would cause problems
in your first week of programming: that what you type in your code determines
whether or not it will perform correctly.

Shouldn't they focus on examples of unexpected or non-obvious mistakes (or at
least start with those -- but then why add obvious examples at the end)?

Whether you copy and paste or simply type a single line incorrectly, of course
if the wrong thing is entered the code will simply be incorrect.

~~~
oneeyedpigeon
You're missing the point: repeated blocks of code (whether they are
copy+pasted or typed out) are simply bad, not from a computing point of view,
in terms of what instructions get executed when, but from a human point of
view, in terms of how we read and comprehend code. It's dealing with a
different aspect of computer programming, rather than the obvious 'what works,
what's efficient', but that makes it _more_ interesting, not less.

~~~
jjallen
Aren't switch-case statements widely accepted in C++ and inherently repeated
blocks of code? No one would tell you to not copy-paste and then edit just the
different parts in a switch-case block.

And to support both our arguments -- that example first mentions copy-pasting
being the likely cause of the error and then provides a rewritten example that
doesn't use the same code repeatedly (your point).

But a switch-case statement is a counter-example that you would copy-paste
code and isn't abstracted into variable array indexes.

Counter-counter-argument: switch-cases have more obvious differences in a
(only potentially) multi-character string.

~~~
ktRolster
_Aren 't switch-case statements widely accepted in C++_

No. Some people say they're a sign of a poorly architected object model.

~~~
ycombobreaker
Both of those statements can be true, though. Switch-case statements are
widely accepted in industry, yet some developers disagree with their use. In
my opinion, it's hard to beat a jump table for some operations, which a switch
block neatly performs. And there's a rational continuum between boolean
conditionals, multi-state (>2) conditionals, and polymophism, such that i
think "some people"'s assertion sounds like throwing out the baby with the
bathwater.

