Re: [PATCH 1/3] http.c: introduce `set_long_from_env()` for convenience
From: Patrick Steinhardt <hidden>
Date: 2025-03-19 16:00:52
On Tue, Mar 18, 2025 at 06:21:34PM -0400, Taylor Blau wrote:
In 7059cd99fc (http_init(): Fix config file parsing, 2009-03-09), http.c gained a new "set_from_env()" function as a convenience function around conditionally assigning an environment variable to some variable if and only if the environment variable was set to begin with. But prior to 7059cd99fc, there were two spots which need to first strtol() whatever is set in the environment before assigning it to a long pointer. Both instances stored the result of getenv() in a temporary variable, and conditionally strtol() it depending on whether or not getenv() returned NULL. Replace those two instances with a new cousin of 'set_from_env()' called 'set_long_from_env()', which does what its name suggests. This allows us to remove the temporary variables and clean up some minor code duplication. More importantly, however, it prepares us for a future commit which will introduce more instances of assigning an environment variable to a long.
Okay, makes sense.
quoted hunk ↗ jump to hunk
Signed-off-by: Taylor Blau <redacted> --- http.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)diff --git a/http.c b/http.c index 0c9a872809..be564fd520 100644 --- a/http.c +++ b/http.c@@ -1256,10 +1256,15 @@ static void set_from_env(char **var, const char *envname) } } +static void set_long_from_env(long *var, const char *envname) +{ + const char *val = getenv(envname); + if (val) + *var = strtol(val, NULL, 10); +}
Hm. We don't perform any error checking at all for whether or not the value of the environment variable is a valid integer. This isn't a new issue introduced by your patch, but now that we have a central place where it's being parsed I wonder whether we should be checking for errors? Patrick