Re: [PATCH v4 2/2] timezone: Replace Localtime file copy with symbolic link

Denis Kenzior <[email protected]> Thu, 5 Jun 2025 09:58:00 -0500
Newsgroups dev.linux.lists.connman
Message-ID <[email protected]>
Hi Andrea,

On 6/5/25 3:03 AM, Andrea Ricchi wrote:
> Based on the `man 5 localtime` documentation the `/etc/localtime`
> file is expected to be a symbolic link, not a standalone
> regular file.
> 
> This change replaces `write_file` with `write_symlink`, avoiding
> unnecessary file I/O and memory mapping. It ensures Localtime accurately
> reflects the selected timezone without duplicating data. This also
> fixes `__connman_timezone_lookup()`, which requires Localtime to be a
> symlink to function correctly.
> ---
>   src/timezone.c | 50 +++++++++++---------------------------------------
>   1 file changed, 11 insertions(+), 39 deletions(-)
> 

<snip>

> @@ -537,35 +527,17 @@ static int write_file(void *src_map, struct stat *src_st, const char *pathname)
>   int __connman_timezone_change(const char *zone)
>   {
>   	struct stat st;
> -	char *map, pathname[PATH_MAX];
> -	int fd, err;
> +	g_autofree char *pathname = NULL;
> +	int err;

Amended this commit to remove 'err' variable in order to fix the warning this 
patch introduced:

[denkenz@archdev connman]$ make
make --no-print-directory all-am
   CC       src/tools_dnsproxy_standalone-timezone.o
src/timezone.c: In function ‘__connman_timezone_change’:
src/timezone.c:531:13: error: unused variable ‘err’ [-Werror=unused-variable]
   531 |         int err;
       |             ^~~
cc1: all warnings being treated as errors


>   
>   	DBG("zone %s", zone);
>   
> -	snprintf(pathname, PATH_MAX, "%s/%s", USR_SHARE_ZONEINFO, zone);
> +	pathname = g_strdup_printf("%s/%s", USR_SHARE_ZONEINFO, zone);
>   
> -	fd = open(pathname, O_RDONLY | O_CLOEXEC);
> -	if (fd < 0)
> +	if (stat(pathname, &st) < 0 || !S_ISREG(st.st_mode))
>   		return -EINVAL;
>   
> -	if (fstat(fd, &st) < 0) {
> -		close(fd);
> -		return -EIO;
> -	}
> -
> -	map = mmap(0, st.st_size, PROT_READ, MAP_SHARED, fd, 0);
> -	if (!map || map == MAP_FAILED) {
> -		close(fd);
> -		return -EIO;
> -	}
> -
> -	err = write_file(map, &st, connman_setting_get_string("Localtime"));
> -
> -	munmap(map, st.st_size);
> -
> -	close(fd);
> -
> -	return err;
> +	return write_symlink(pathname, connman_setting_get_string("Localtime"));
>   }
>   
>   static guint inotify_watch = 0;

Applied, thanks.

Regards,
-Denis