@matheusbsbs / matheus-bagliano / commits / 05e293c1ac

config: move flockfile() closer to unlocked functions

Commit 260d408e32 (config: use getc_unlocked when reading from file, 2015-04-16) taught git_config_from_file() to lock the filehandle so that we could safely use the faster unlocked functions to access the handle. However, it split the logic into two places: 1. The master lock/unlock happens in git_config_from_file(). 2. The decision to use the unlocked functions happens in do_config_from_file(). That means that if anybody calls the latter function, they will accidentally use the unlocked functions without holding the lock. And indeed, git_config_from_stdin() does so. In practice, this hasn't been a problem since this code isn't generally multi-threaded (and even if some Git program happened to have another thread running, it's unlikely to be reading from stdin). But it's a good practice to make sure we're always holding the lock before using the unlocked functions. Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Mar 30, 2018 at 15:26 UTC 05e293c1ac63295fcfe1b8fd9533fcdb012d5f03

1 file changed +5 -3

config.c

+5 -3

@@ -1408,6 +1408,7 @@ static int do_config_from_file(config_fn_t fn,

1408 void *data)

1409 {

1410 struct config_source top;

1411 + int ret;

1412

1413 top.u.file = f;

1414 top.origin_type = origin_type;

@@ -1418,7 +1419,10 @@ static int do_config_from_file(config_fn_t fn,

1419 top.do_ungetc = config_file_ungetc;

1420 top.do_ftell = config_file_ftell;

1421

1421 - return do_config_from(&top, fn, data);

1422 + flockfile(f);

1423 + ret = do_config_from(&top, fn, data);

1424 + funlockfile(f);

1425 + return ret;

1426 }

1427

1428 static int git_config_from_stdin(config_fn_t fn, void *data)

@@ -1433,9 +1437,7 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)

1437

1438 f = fopen_or_warn(filename, "r");

1439 if (f) {

1436 - flockfile(f);

1440 ret = do_config_from_file(fn, CONFIG_ORIGIN_FILE, filename, filename, f, data);

1438 - funlockfile(f);

1441 fclose(f);

1442 }

1443 return ret;