Skip to content

general code-cleanup#479

Merged
arogge merged 70 commits intobareos:masterfrom
arogge:dev/arogge/master/cleanup
May 25, 2020
Merged

general code-cleanup#479
arogge merged 70 commits intobareos:masterfrom
arogge:dev/arogge/master/cleanup

Conversation

@arogge
Copy link
Member

@arogge arogge commented Apr 9, 2020

  • migrate host.h and friends into config.h
  • remove some unused definitions from config.h and their cmake counterparts
  • allow usage of FORTIFY_SOURCE (which will generate a lot of additional warnings)
  • introduce new macro INTENDED_FALLTHROUGH
  • fix some old and new compiler warnings

When reviewing, looking at the individual commits is probably easiest.

TODO:

  • remove even more definitions from config.h
  • remove all unused checks from BareosCheckFunctions.cmake
  • maybe move config.h into CMAKE_BINARY_DIR

Copy link
Contributor

@franku franku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job!

@arogge arogge force-pushed the dev/arogge/master/cleanup branch 3 times, most recently from 1f2de9f to 6584f19 Compare April 15, 2020 11:28
@arogge arogge force-pushed the dev/arogge/master/cleanup branch 6 times, most recently from f29e671 to b76ab23 Compare April 30, 2020 13:56
@arogge arogge force-pushed the dev/arogge/master/cleanup branch from f899545 to 91e85cb Compare May 6, 2020 14:42
Copy link
Contributor

@franku franku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really a huge work, thanks for that!
I have some really minor suggestions and a list of typos in a private message..

for (list ? (*((void**)&(var)) = (void*)((list)->first())) : NULL; (var); \
(*((void**)&(var)) = (void*)((list)->next())))
#define foreach_alist_null(var, list) \
for ((void)(list ? (*((void**)&(var)) = (void*)((list)->first())) : NULL); \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for ((void)(list ? (*((void**)&(var)) = (void*)((list)->first())) : NULL); \
for ((void)(list ? (*((void**)&(var)) = (void*)((list)->first())) : nullptr); \

return mdb;
}

#ifdef HAVE_SQL_POOLING
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Krass, wie viel Zeit in ungenutzem Code verheizt wurde..


set(HAVE_CRYPTO 1)

include(BareosTypeSizes)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole commit is one of the coolest I have seen on this code. ;-) Very good, now we have stdint.h.

@arogge arogge force-pushed the dev/arogge/master/cleanup branch from b3025e0 to 14ed933 Compare May 12, 2020 09:13
@arogge
Copy link
Member Author

arogge commented May 12, 2020

Rebased to latest master and spell-checked.

@arogge arogge marked this pull request as ready for review May 12, 2020 09:14
@arogge arogge force-pushed the dev/arogge/master/cleanup branch from 41ba2e1 to a020d3d Compare May 12, 2020 12:20
arogge added 14 commits May 25, 2020 10:38
* merge host.h and friends into config.h
 remove unused defines from config.h
cmake generated two git information files. As these are not used anymore
this patch removes the call and the associated script.
When we used SmartAlloc we could not allow FORTIFY_SOURCE, so it was
disabled on purpose. Now that SmartAllos is gone, we can allow
FORTIFY_SOURCE and this patch removes the #undef from bareos.h.
Enabling FORTIFY_SOURCE yielded a lot of new warnings. In case of
intentionally ignored return-values or expressions these are now
silenced.
Other warnings have been fixed.
lmdb's mdb.c contains a construct that triggers a missing-braces
warning. This patch adds the -Wno-missing-braces option when building
lmdb to silence this warning.
This patch adds a new macro FALLTHROUGH_INTENDED that will evaluate to
the correct fallthrough attribute depending on the compiler used.
An unused return-code has been stored in a then unused variable when
invoking getcwd(). This patch simply ignores the return-value instead.
The removal of the git informational files from the build process also
removed the build/ directory from the source-tree.
This patch now removes the reference to the removed directory from
Debian packaging.
strtoll() is C99, so we expect it to be always present.
As bcopy() is marked as LEGACY in POSIX.1-2001 we will simply use
memmove() in all cases instead.
This function was only used to generate a session key if it was
available. As the session-key will probably not suffer from not adding
this gethostid() can probably safely removed.
This may have been useful in the ancient past, but modern operating
systems are really good at memory handling and usually don't need much
help to run things optimally. Thus calling malloc_trim() is probably not
a great idea anymore.
arogge added 28 commits May 25, 2020 10:39
This type can be safely assumed to exist.
This patch removes the definitions of lld and llu, that were rarely used
and could simply be replaced by "lld" and "llu".
Every identifier starting with an underscore and continuing with an
uppercase letter is reserved and its use is undefined behaviour.
This patch renames the identifiers _PATH_BAREOS_BACKENDDIR,
_PATH_BAREOS_PIDDIR and _PATH_BAREOS_WORKINGDIR to not contain a leading
underscore.
these functions are required by the standard and can be assumed to
exist.
The DISTNAME contains a Bareos-specific short-name of the platform that
is built. As such it is not the DISTNAME itself, but the Bareos-specific
platform identifier.
This patch now renames DISTNAME to PLATFORM.
The plugin interface allowed to query the platform, distribution and
version of the filedaemon using bVarDistName. This has not been used by
any plugin and the provided value was of questionable quality.
This patch removes support for bVarDistName. Querying it will now raise
an error. The related dist_name from message.cc that was used solely for
this purpose has also been removed.
For the sole purpose of printing its value on a traceback there is a
constant string in message.cc that contains the built-time system
information.
This patch removes the constant and its use in the backtrace scripts.
This patch adds a new function GetOsInfo() to kBareosVersionStrings. In
the future this function will be used as a replacement of the DISTVER
definition.
Eventually the GetOsInfo() should retrieve the information at runtime
instead of having it compiled in.

This patch also replaces DISTVER with a call to
kBareosVersionStrings.GetOsInfo() where possible. For windows this means
that we now always report the information obtained at runtime when the
program started.
Previously the function GetWindowsVersionString was used in some places
to determine the running windows version.
This patch removes the function and its usage. It is replaced by the
os-independent GetOsInfoString() and kBareosVersionStrings.GetOsInfo().
Its functionality has been moved into the windows-specific
implementation of the new function.
previously the list of jobs was not sorted when the prune volume code
ran. This patch sorts the list so the log message and SQL query are
more predictable.
This also changes some prototypes to const where possible (and required
by std::string).
Lastly we add a "wait" in the volume-pruning-test to make sure the
previous jobs have finished before we start the jobs that prunes.
The catalog configuration for the system:catalog test was missing the
mysql variant.
This patch adds the required resource so the test works on mysql, too.
Previously a lot of the addresses in the systemtest configurations have
been set to localhost. This patch now uses @hostname@ everywhere so
addressing is consistent.
Previously once setup by the setup-script the catalog for the systemtest
was set to a specific database type. This made it impossible to test
different database backends without reconfiguring the tests in between.
This patch now always resets the dbdriver setting in the catalog
configuration so setup always changes dbdriver to the correct value.
in win32's compat.cc there was another function that detects the windows
version. The result of this would only used when building with visual
studio and is redundant as we already have osinfo_win32.cc.
This patch removes that duplicate code.
Also fix a compiler warning in dlist.h so pragmas can be reduced
disable unused-variable warnings in rpc code when building with clang.
* unused function swap() in crc32
* braces instead of parentheses for initialization
The macro was only used once and could be replaced by Emsg1(M_ABORT,...)
and as such was unnecessary.
This patch removes the macro and related code.
Previously, Bareos used intentional SEGFAULTs to terminate the program
when a fatal error occurs.
This patch replaces these with calls to abort() that will terminate the
program using SIGABRT.
The core for tcp-wrappers has been disabled since the switch to cmake.
This patch removes the dead code and build-parameters
When using strncpy() the terminating NULL could be truncated. By
asserting the length of the source string is short enough, the code is
safe and the warning is silenced.
A lot of stored locking code was duplicated and could be used by setting
SD_DEBUG_LOCK. This was not used and was not settable via cmake, so this
patch removes the definition and the #ifdef'ed code.
@arogge arogge force-pushed the dev/arogge/master/cleanup branch from a020d3d to ae6bb57 Compare May 25, 2020 08:44
@arogge arogge merged commit ca5d97b into bareos:master May 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants