Modularize FTL modules/headers #605

DL6ER · 2019-07-01T17:43:32Z

By submitting this pull request, I confirm the following (please check boxes, eg [X]) Failure to fill the template will close your PR:

Please submit all pull requests against the development branch. Failure to do so will delay or deny your request

I have read and understood the contributors guide.
I have checked that another pull request for this purpose does not exist.
I have considered, and confirmed that this submission will be valuable to others.
I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
I give this submission freely, and claim no ownership to its content.

How familiar are you with the codebase?:

10

Since the advent of our Faster Than Light (pihole-FTL) engine, it somewhat organically grew and quickly became a complex daemon. However, instead of properly fractionating the code into clearly separated modules, we instead got two very large files FTL.h and routines.h which contained prototypes for everything.

This PR restructures our FTL engine by separating the declarations and functions into separated modules, each consisting of a .c and the corresponding .h files. These two files build modules, each corresponding to a clear piece of functionality. They can be largely maintained and developed separately from the other modules, and partially even reused in entirely different projects.

Summary of the changes in this pull request:

Move definitions of global variables from FTL.h into the corresponding module headers where this is practical.
Move all prototypes into the corresponding module headers. This allows us to remove the old and very large routines.h altogether.
Move database related modules into a common subdirectory database/
Split the old database.c file into two smaller files (database/common.c and database/query-table.c).
Rename some files and functions for the sake of consistency.
Move some files from their old module into a new one if they fit there better. An example is moving the function that calls stat() to obtain the physical size of the database from database.c into files.c where similar functions already exist.

Signed-off-by: DL6ER <dl6er@dl6er.de>

…utines.h Signed-off-by: DL6ER <dl6er@dl6er.de>

Signed-off-by: DL6ER <dl6er@dl6er.de>

…ar as this is applicable (we do not do it for the thread pointers as this complicates linking dependencies). Signed-off-by: DL6ER <dl6er@dl6er.de>

Signed-off-by: DL6ER <dl6er@dl6er.de>

…ble.o Signed-off-by: DL6ER <dl6er@dl6er.de>

Signed-off-by: DL6ER <dl6er@dl6er.de>

…-the-fly. Signed-off-by: DL6ER <dl6er@dl6er.de>

…fix clean target. Signed-off-by: DL6ER <dl6er@dl6er.de>

Signed-off-by: DL6ER <dl6er@dl6er.de>

…ts into new subdirectory "build". Signed-off-by: DL6ER <dl6er@dl6er.de>

Signed-off-by: DL6ER <dl6er@dl6er.de>

…rectory src/api. Signed-off-by: DL6ER <dl6er@dl6er.de>

Signed-off-by: DL6ER <dl6er@dl6er.de>

Makefile

src/files.c

src/config.h

src/database/common.c

src/database/network-table.c

src/files.h

src/timers.c

.gitignore

Signed-off-by: DL6ER <dl6er@dl6er.de>

…nfig{.c/.h} Signed-off-by: DL6ER <dl6er@dl6er.de>

Signed-off-by: DL6ER <dl6er@dl6er.de>

… prefer to handle the logging through the SQLite3 error callback as this ensures it cannot be missed. Previously: [2019-07-06 17:00:55.526 32339] SQLite3 message: statement aborts at 36: [CREATE UNIQUE INDEX network_hwaddr_idx ON network(hwaddr)] attempt to write a readonly database (8) [2019-07-06 17:00:55.526 32339] dbquery(CREATE UNIQUE INDEX network_hwaddr_idx ON network(hwaddr)) - SQL error (8): attempt to write a readonly database [2019-07-06 17:00:55.526 32339] check_database(8): Disabling database connection due to error [2019-07-06 17:00:55.526 32339] unify_hwaddr(): "CREATE UNIQUE INDEX network_hwaddr_idx ON network(hwaddr)" failed! Now: [2019-07-06 17:00:55.526 32339] SQLite3 message: statement aborts at 36: [CREATE UNIQUE INDEX network_hwaddr_idx ON network(hwaddr)] attempt to write a readonly database (8) [2019-07-06 17:00:55.526 32339] check_database(8): Disabling database connection due to error [2019-07-06 17:00:55.526 32339] ERROR: unify_hwaddr() failed! Signed-off-by: DL6ER <dl6er@dl6er.de>

… error. Signed-off-by: DL6ER <dl6er@dl6er.de>

Signed-off-by: DL6ER <dl6er@dl6er.de>

src/setupVars.h

src/timers.h

…e we set database to false. Signed-off-by: DL6ER <dl6er@dl6er.de>

…ld_queries_in_DB(). The function can in any way not be called when database == false as this is an exit criterion for the database thread. Signed-off-by: DL6ER <dl6er@dl6er.de>

Signed-off-by: DL6ER <dl6er@dl6er.de>

…commented. Signed-off-by: DL6ER <dl6er@dl6er.de>

src/dnsmasq_interface.c

Database error checking

… that are not needed. Signed-off-by: DL6ER <dl6er@dl6er.de>

AzureMarker · 2019-07-09T03:09:24Z

Note that the codefactor issues are ignored, but the GitHub check is not updated.

DL6ER added 13 commits June 29, 2019 18:01

Modularize header files

7a100ae

Signed-off-by: DL6ER <dl6er@dl6er.de>

Only include smaller header files in the source code files. Remove ro…

602bd07

…utines.h Signed-off-by: DL6ER <dl6er@dl6er.de>

Mover timer functions in separate C file

c212c26

Signed-off-by: DL6ER <dl6er@dl6er.de>

Combine shmem.h and shmem_r.h

b02c69f

Signed-off-by: DL6ER <dl6er@dl6er.de>

Add overTime.h header

ff12c1b

Signed-off-by: DL6ER <dl6er@dl6er.de>

Move global variable declarations into the suitable header files as f…

55ed61f

…ar as this is applicable (we do not do it for the thread pointers as this complicates linking dependencies). Signed-off-by: DL6ER <dl6er@dl6er.de>

Ignore any database files that might be present in the repository.

1d0dc1d

Signed-off-by: DL6ER <dl6er@dl6er.de>

Move database modules into common subdirectory

2115cdc

Signed-off-by: DL6ER <dl6er@dl6er.de>

Rename database/database.c => database/common.c

57116ae

Signed-off-by: DL6ER <dl6er@dl6er.de>

Move query table related routines into a new module database/query-ta…

4db159d

…ble.o Signed-off-by: DL6ER <dl6er@dl6er.de>

Rename query reading/saving routines for consistency.

67e684b

Signed-off-by: DL6ER <dl6er@dl6er.de>

Simplify get_number_of_queries_in_DB().

b6d4e5e

Signed-off-by: DL6ER <dl6er@dl6er.de>

Rename database/{gravity.* => gravity-db.*}

49a009c

Signed-off-by: DL6ER <dl6er@dl6er.de>

DL6ER added the Code maintanance label Jul 1, 2019

DL6ER added this to the v5.0 milestone Jul 1, 2019

DL6ER added 10 commits July 1, 2019 19:46

gravity.c has been renamed to gravity-db.c

5eaf184

Signed-off-by: DL6ER <dl6er@dl6er.de>

Use local sqlite3.h

54547b9

Signed-off-by: DL6ER <dl6er@dl6er.de>

Readd version.h as explicit dependency in order to have it created on…

4556b97

…-the-fly. Signed-off-by: DL6ER <dl6er@dl6er.de>

Remove dependency of api.c on the huge header database/sqlite3.h and …

5a41241

…fix clean target. Signed-off-by: DL6ER <dl6er@dl6er.de>

Rename object dir for dnsmasq files in Makefile.

6b08566

Signed-off-by: DL6ER <dl6er@dl6er.de>

Fix wrong standard filename for FTL\'s database.

0003124

Signed-off-by: DL6ER <dl6er@dl6er.de>

Move all source files into new subdirectory "src". Put compiled objec…

e365364

…ts into new subdirectory "build". Signed-off-by: DL6ER <dl6er@dl6er.de>

Merge branch 'development' into new/modularize_headers

a2961d1

Signed-off-by: DL6ER <dl6er@dl6er.de>

Move API modules (api, request, socket, msgpack) into their own subdi…

6b2f22f

…rectory src/api. Signed-off-by: DL6ER <dl6er@dl6er.de>

Move database thread into dedicated module.

104ea1b

Signed-off-by: DL6ER <dl6er@dl6er.de>

AzureMarker reviewed Jul 4, 2019

View reviewed changes

DL6ER added 4 commits July 5, 2019 11:58

Merge branch 'development' into new/modularize_headers

ad27f6f

Make variables in Makefile more readable by inserting underscores.

5699252

Signed-off-by: DL6ER <dl6er@dl6er.de>

Change type of get_FTL_db_filesize() from double to long int.

8280f45

Signed-off-by: DL6ER <dl6er@dl6er.de>

Also enforce recompilation on change of headers in the subdirectories.

ee671a1

Signed-off-by: DL6ER <dl6er@dl6er.de>

DL6ER added 7 commits July 5, 2019 12:08

Move definition of config struct into src/config.h.

29dd190

Signed-off-by: DL6ER <dl6er@dl6er.de>

Remove extra space.

40a75c2

Signed-off-by: DL6ER <dl6er@dl6er.de>

Move blocking status detection from files.c to setupVars.c

25161c1

Signed-off-by: DL6ER <dl6er@dl6er.de>

Define timers in timers.h.

3ce04f0

Signed-off-by: DL6ER <dl6er@dl6er.de>

The macvendor script has moved into subdirectory tools.

a4e50c3

Signed-off-by: DL6ER <dl6er@dl6er.de>

Union FTLFileNamesStruct and logFileNamesStruct and define them in co…

e13e39a

…nfig{.c/.h} Signed-off-by: DL6ER <dl6er@dl6er.de>

Fix incorrect INSERT statement in gravity.db test data.

9f127e9

Signed-off-by: DL6ER <dl6er@dl6er.de>

DL6ER mentioned this pull request Jul 5, 2019

Use database table for domain auditlog #608

Merged

5 tasks

DL6ER added 4 commits July 6, 2019 17:03

Improve error handling and logging in database/common.c

8baf8e6

Signed-off-by: DL6ER <dl6er@dl6er.de>

Do not assume the database is not available before we actually hit an…

444255b

… error. Signed-off-by: DL6ER <dl6er@dl6er.de>

No need to use heap space for a constant expression.

71d9caa

Signed-off-by: DL6ER <dl6er@dl6er.de>

AzureMarker reviewed Jul 6, 2019

View reviewed changes

src/setupVars.h Show resolved Hide resolved

src/timers.h Show resolved Hide resolved

DL6ER added 5 commits July 7, 2019 09:45

Move checking of database parameter MAXDBDAYS into db_init() to ensur…

4a9503c

…e we set database to false. Signed-off-by: DL6ER <dl6er@dl6er.de>

The boolean database should not be set to true at the end of delete_o…

812743c

…ld_queries_in_DB(). The function can in any way not be called when database == false as this is an exit criterion for the database thread. Signed-off-by: DL6ER <dl6er@dl6er.de>

Move even more definitions from FTL.h into the respective headers.

5c75fc3

Signed-off-by: DL6ER <dl6er@dl6er.de>

Remove obsolete comment.

e30df58

Signed-off-by: DL6ER <dl6er@dl6er.de>

Added more comments to dnsmasq-interface.c. Now almost every line is …

867e466

…commented. Signed-off-by: DL6ER <dl6er@dl6er.de>

AzureMarker reviewed Jul 7, 2019

View reviewed changes

src/dnsmasq_interface.c Outdated Show resolved Hide resolved

AzureMarker and others added 2 commits July 7, 2019 15:04

Merge pull request #610 from pi-hole/tweak/database_error_checking

ac1018a

Database error checking

Simplify logic in detect_blocked_IP(), remove extra return statements…

b3ad13f

… that are not needed. Signed-off-by: DL6ER <dl6er@dl6er.de>

AzureMarker approved these changes Jul 9, 2019

View reviewed changes

AzureMarker merged commit 5e85489 into development Jul 9, 2019

AzureMarker deleted the new/modularize_headers branch July 9, 2019 03:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modularize FTL modules/headers #605

Modularize FTL modules/headers #605

DL6ER commented Jul 1, 2019

AzureMarker commented Jul 9, 2019

Modularize FTL modules/headers #605

Modularize FTL modules/headers #605

Conversation

DL6ER commented Jul 1, 2019

10

AzureMarker commented Jul 9, 2019