33 lines
1.5 KiB
Text
33 lines
1.5 KiB
Text
- There is way too much of busy waiting for fairly arbitrarily chosen
|
|
timeout periods in the code.
|
|
|
|
With "busy wait" here I of course don't mean *real* busy wait that
|
|
would actually mindlessly spin the CPU.
|
|
|
|
I mean calling poll() with some short timeout (order of seconds),
|
|
and each time a timeout happens (which surely is the large majority
|
|
of times), check a condition, which in the large majority of times
|
|
will not be true, and then repeat.
|
|
|
|
Instead the code could use for instance eventfds to enable the
|
|
poll() calls to be without timeout. Or something similar, depending
|
|
on case.
|
|
|
|
- Recursive mutexes are evil. In general, I think the consensus is
|
|
that recursive mutexes should be avoided. One should use them only
|
|
when absolutely necessary because the code-base is so complex that
|
|
one has no idea how it works. That was hopefully not the case when
|
|
recursive mutexes were introduced here? But probably it is by now...
|
|
|
|
- Occasionally Control-C (SIGINT) doesn't shut fown loolwsd. One has
|
|
to kill it with SIGKILL. Which of course leaves all the chroot jails
|
|
around.
|
|
|
|
- There are lots of places where a std::string variable is defined,
|
|
initialised with a value, that is never changed. (In many cases it
|
|
is const, so could of course not be changed.) Then the variable is
|
|
used just once or twice, passed as a parameter to a function, or
|
|
used in a comparisin expression. This is fairly pointless and just
|
|
makes the code harder to read. Use string literals instead.
|
|
|
|
- ASCII art? Seriously?
|