Skip to content

Commit e5a3fe7

Browse files
hummeltechhummeltech
committed
Add renderd_config_test helper, consolidate redundant code in mod_tile.c, stop unnecessarily reading renderd.conf 3 times, improve testing
1 parent 51fabcb commit e5a3fe7

8 files changed

Lines changed: 296 additions & 83 deletions

File tree

‎Makefile.am‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ bin_PROGRAMS = \
2626
render_speedtest
2727
noinst_PROGRAMS = \
2828
gen_tile_test \
29+
renderd_config_test_helper \
2930
renderd_config_test \
3031
renderd_test \
3132
render_expired_test \
@@ -109,6 +110,12 @@ gen_tile_test_CFLAGS = -DMAIN_ALREADY_DEFINED
109110
gen_tile_test_CXXFLAGS = $(renderd_CXXFLAGS)
110111
gen_tile_test_LDADD = $(renderd_LDADD) catch_test_common.o
111112

113+
renderd_config_test_helper_SOURCES = \
114+
tests/renderd_config_test_helper.cpp \
115+
src/g_logger.c \
116+
src/renderd_config.c
117+
renderd_config_test_helper_LDADD = $(GLIB_LIBS) $(INIPARSER_LDFLAGS) catch_main.o
118+
112119
renderd_config_test_SOURCES = \
113120
tests/renderd_config_test.cpp
114121
renderd_config_test_LDADD = $(GLIB_LIBS) catch_main.o catch_test_common.o
@@ -137,7 +144,7 @@ CLEANFILES=*.slo mod_tile.la stderr.out src/*.slo src/*.lo src/.libs/* src/*.la
137144

138145
COMMA=,
139146

140-
test: gen_tile_test renderd_config_test renderd_test render_expired_test render_list_test render_old_test render_speedtest_test
147+
test: gen_tile_test renderd_config_test_helper renderd_config_test renderd_test render_expired_test render_list_test render_old_test render_speedtest_test
141148
./gen_tile_test
142149
./renderd_config_test
143150
./renderd_test

‎includes/renderd_config.h‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@
2121
#include "render_config.h"
2222
#include "renderd.h"
2323

24+
#ifdef HAVE_INIPARSER_INIPARSER_H
25+
#include <iniparser/iniparser.h>
26+
#else
27+
#include <iniparser.h>
28+
#endif
29+
2430
#ifdef __cplusplus
2531
extern "C" {
2632
#endif
@@ -37,9 +43,9 @@ void free_map_sections(xmlconfigitem *map_sections);
3743
void free_renderd_section(renderd_config renderd_section);
3844
void free_renderd_sections(renderd_config *renderd_sections);
3945
void process_config_file(const char *config_file_name, int active_renderd_section_num, int log_level);
40-
void process_map_sections(const char *config_file_name, xmlconfigitem *maps_dest, const char *default_tile_dir, int num_threads);
41-
void process_mapnik_section(const char *config_file_name, renderd_config *config_dest);
42-
void process_renderd_sections(const char *config_file_name, renderd_config *configs_dest);
46+
void process_map_sections(dictionary *ini, const char *config_file_name, xmlconfigitem *maps_dest, const char *default_tile_dir, int num_threads);
47+
void process_mapnik_section(dictionary *ini, const char *config_file_name, renderd_config *config_dest);
48+
void process_renderd_sections(dictionary *ini, const char *config_file_name, renderd_config *configs_dest);
4349

4450
#ifdef __cplusplus
4551
}

‎src/CMakeLists.txt‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,4 +219,23 @@ target_include_directories(gen_tile_test PRIVATE ${PROJECT_SOURCE_DIR}/tests)
219219
target_link_libraries(gen_tile_test ${gen_tile_test_LIBS})
220220
set_target_properties(gen_tile_test PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/tests)
221221

222+
#-----------------------------------------------------------------------------
223+
#
224+
# renderd_config_test_helper
225+
#
226+
#-----------------------------------------------------------------------------
227+
228+
set(renderd_config_test_helper_SRCS
229+
${PROJECT_SOURCE_DIR}/tests/renderd_config_test_helper.cpp
230+
g_logger.c
231+
renderd_config.c
232+
)
233+
set(renderd_config_test_helper_LIBS
234+
${GLIB_LIBRARIES}
235+
${INIPARSER_LIBRARIES}
236+
)
237+
add_executable(renderd_config_test_helper ${renderd_config_test_helper_SRCS})
238+
target_link_libraries(renderd_config_test_helper ${renderd_config_test_helper_LIBS})
239+
set_target_properties(renderd_config_test_helper PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/tests)
240+
222241
endif()

‎src/mod_tile.c‎

Lines changed: 39 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,7 @@ static void add_expiry(request_rec *r, struct protocol *cmd)
551551

552552
apr_table_mergen(t, "Cache-Control",
553553
apr_psprintf(r->pool, "max-age=%li", maxAge));
554-
timestr = (char *)apr_palloc(r->pool, APR_RFC822_DATE_LEN);
554+
timestr = (char *)apr_pcalloc(r->pool, APR_RFC822_DATE_LEN);
555555
apr_rfc822_date(timestr, (apr_time_from_sec(maxAge) + r->request_time));
556556
apr_table_setn(t, "Expires", timestr);
557557
}
@@ -1301,7 +1301,7 @@ static int tile_handler_json(request_rec *r)
13011301
}
13021302
}
13031303

1304-
buf = (char *)apr_palloc(r->pool, 8 * 1024);
1304+
buf = (char *)apr_pcalloc(r->pool, 8 * 1024);
13051305

13061306
snprintf(buf, 8 * 1024,
13071307
"{\n"
@@ -1343,22 +1343,41 @@ static int tile_handler_json(request_rec *r)
13431343
ap_set_content_length(r, len);
13441344
apr_table_mergen(t, "Cache-Control",
13451345
apr_psprintf(r->pool, "max-age=%li", maxAge));
1346-
timestr = (char *)apr_palloc(r->pool, APR_RFC822_DATE_LEN);
1346+
timestr = (char *)apr_pcalloc(r->pool, APR_RFC822_DATE_LEN);
13471347
apr_rfc822_date(timestr, (apr_time_from_sec(maxAge) + r->request_time));
13481348
apr_table_setn(t, "Expires", timestr);
13491349
ap_rwrite(buf, len, r);
13501350

13511351
return OK;
13521352
}
13531353

1354+
static int _get_stats_copy(request_rec *r, tile_server_conf *scfg, stats_data **stats_copy) {
1355+
stats_data *stats;
1356+
unsigned long sizeof_config_elements;
1357+
1358+
if (get_global_lock(r, stats_mutex) != 0) {
1359+
// Copy over the global counter variable into
1360+
// local variables, that we can immediately
1361+
// release the lock again
1362+
sizeof_config_elements = sizeof(apr_uint64_t) * scfg->configs->nelts;
1363+
stats = (stats_data *)apr_shm_baseaddr_get(stats_shm);
1364+
*stats_copy = (stats_data *)apr_pmemdup(r->pool, stats, sizeof(stats_data));
1365+
(*stats_copy)->noResp200Layer = (apr_uint64_t *)apr_pmemdup(r->pool, stats->noResp200Layer, sizeof_config_elements);
1366+
(*stats_copy)->noResp404Layer = (apr_uint64_t *)apr_pmemdup(r->pool, stats->noResp404Layer, sizeof_config_elements);
1367+
apr_global_mutex_unlock(stats_mutex);
1368+
} else {
1369+
return error_message(r, "Failed to acquire lock, can't copy stats");
1370+
}
1371+
1372+
return OK;
1373+
}
1374+
13541375
static int tile_handler_mod_stats(request_rec *r)
13551376
{
1356-
stats_data *stats;
1357-
stats_data *local_stats;
13581377
int i;
1359-
unsigned long sizeof_config_elements;
1360-
tile_server_conf *scfg;
1378+
stats_data *local_stats;
13611379
tile_config_rec *tile_configs;
1380+
tile_server_conf *scfg;
13621381

13631382
if (strcmp(r->handler, "tile_mod_stats")) {
13641383
return DECLINED;
@@ -1371,18 +1390,10 @@ static int tile_handler_mod_stats(request_rec *r)
13711390
return error_message(r, "Stats are not enabled for this server");
13721391
}
13731392

1374-
if (get_global_lock(r, stats_mutex) != 0) {
1375-
// Copy over the global counter variable into
1376-
// local variables, that we can immediately
1377-
// release the lock again
1378-
sizeof_config_elements = sizeof(apr_uint64_t) * scfg->configs->nelts;
1379-
stats = (stats_data *)apr_shm_baseaddr_get(stats_shm);
1380-
local_stats = (stats_data *)apr_pmemdup(r->pool, stats, sizeof(stats_data));
1381-
local_stats->noResp200Layer = (apr_uint64_t *)apr_pmemdup(r->pool, stats->noResp200Layer, sizeof_config_elements);
1382-
local_stats->noResp404Layer = (apr_uint64_t *)apr_pmemdup(r->pool, stats->noResp404Layer, sizeof_config_elements);
1383-
apr_global_mutex_unlock(stats_mutex);
1384-
} else {
1385-
return error_message(r, "Failed to acquire lock, can't display stats");
1393+
_get_stats_copy(r, scfg, &local_stats);
1394+
1395+
if (!local_stats) {
1396+
return OK;
13861397
}
13871398

13881399
ap_rprintf(r, "NoResp200: %" APR_UINT64_T_FMT "\n", local_stats->noResp200);
@@ -1421,12 +1432,10 @@ static int tile_handler_mod_stats(request_rec *r)
14211432

14221433
static int tile_handler_metrics(request_rec *r)
14231434
{
1424-
stats_data *stats;
1425-
stats_data *local_stats;
14261435
int i;
1427-
unsigned long sizeof_config_elements;
1428-
tile_server_conf *scfg;
1436+
stats_data *local_stats;
14291437
tile_config_rec *tile_configs;
1438+
tile_server_conf *scfg;
14301439

14311440
if (strcmp(r->handler, "tile_metrics")) {
14321441
return DECLINED;
@@ -1439,18 +1448,10 @@ static int tile_handler_metrics(request_rec *r)
14391448
return error_message(r, "Stats are not enabled for this server");
14401449
}
14411450

1442-
if (get_global_lock(r, stats_mutex) != 0) {
1443-
// Copy over the global counter variable into
1444-
// local variables, that we can immediately
1445-
// release the lock again
1446-
sizeof_config_elements = sizeof(apr_uint64_t) * scfg->configs->nelts;
1447-
stats = (stats_data *)apr_shm_baseaddr_get(stats_shm);
1448-
local_stats = (stats_data *)apr_pmemdup(r->pool, stats, sizeof(stats_data));
1449-
local_stats->noResp200Layer = (apr_uint64_t *)apr_pmemdup(r->pool, stats->noResp200Layer, sizeof_config_elements);
1450-
local_stats->noResp404Layer = (apr_uint64_t *)apr_pmemdup(r->pool, stats->noResp404Layer, sizeof_config_elements);
1451-
apr_global_mutex_unlock(stats_mutex);
1452-
} else {
1453-
return error_message(r, "Failed to acquire lock, can't display stats");
1451+
_get_stats_copy(r, scfg, &local_stats);
1452+
1453+
if (!local_stats) {
1454+
return OK;
14541455
}
14551456

14561457
ap_rprintf(r, "# HELP modtile_http_responses_total Number of HTTP responses by response code\n");
@@ -1554,7 +1555,7 @@ static int tile_handler_serve(request_rec *r)
15541555
gettimeofday(&start, NULL);
15551556

15561557
// FIXME: It is a waste to do the malloc + read if we are fulfilling a HEAD or returning a 304.
1557-
buf = (char *)apr_palloc(r->pool, tile_max);
1558+
buf = (char *)apr_pcalloc(r->pool, tile_max);
15581559

15591560
if (!buf) {
15601561
if (!incRespCounter(HTTP_INTERNAL_SERVER_ERROR, r, cmd, rdata->layerNumber)) {
@@ -1894,7 +1895,7 @@ static const char *_add_tile_config(cmd_parms *cmd,
18941895
tile_dir = apr_pstrndup(cmd->pool, scfg->tile_dir, PATH_MAX);
18951896
}
18961897

1897-
char **hostnames = (char **)malloc(sizeof(char *) * hostnames_len);
1898+
char **hostnames = (char **)apr_pcalloc(cmd->pool, sizeof(char *) * hostnames_len);
18981899

18991900
// Set first hostname to server_hostname value (if set,) otherwise use localhost
19001901
if (cmd->server->server_hostname == NULL) {
@@ -2022,7 +2023,7 @@ static const char *load_tile_config(cmd_parms *cmd, void *mconfig, const char *c
20222023

20232024
xmlconfigitem maps[XMLCONFIGS_MAX];
20242025

2025-
process_map_sections(config_file_name, maps, "", 0);
2026+
process_map_sections(NULL, config_file_name, maps, "", 0);
20262027

20272028
for (int i = 0; i < XMLCONFIGS_MAX; i++) {
20282029
if (maps[i].xmlname != NULL) {

0 commit comments

Comments
 (0)