Hi Max
On 23/03/2024 11:07, Max Gautier wrote:
On Fri, Mar 22, 2024 at 11:11:09PM +0100, Max Gautier wrote:
+/*
+ * TODO: in the future (~2026 ?) remove this cleanup code
+ */
That is rather optimistic - users only run "git maintenance start" once
so any unit files that have been written in the past will exist well
beyond 2026.
+static void systemd_delete_user_unit(char const *unit)
+{
+ char const file_start_stale[] = "# This file was created and is"
+ " maintained by Git.";
+ char file_start_user[sizeof(file_start_stale)] = {'\0'};
+
+ char *filename = xdg_config_home_for("systemd/user", unit);
+ int handle = open(filename, O_RDONLY);
+
+ /*
+ * Check this is actually our file and we're not removing a legitimate
+ * user override.
+ */
+ if (handle == -1 && !is_missing_file_error(errno))
+ warning(_("failed to delete '%s'"), filename);
+ else {
+ read(handle, file_start_user, sizeof(file_start_stale) - 1);
Actually that fails -Werror because I don't check read return.
Alternative below (on top of this one), with one question:
Are VLA using size_t const OK ? It's folded to a constant array by gcc
but I don't know if that causes portability problem with other platforms
? I can always repeat the sizeof expr if it's a problematic construct.
I think it would be easier to use strbuf_read_file() instead - it is
only a small file so there is not really any advantage in just reading
the first line. Something like
static int systemd_delete_user_unit(const char* unit)
{
int ret = 0;
struct strbuf buf = STRBUF_INIT;
char *filename = xdg_config_home_for("systemd/user", unit);
if (strbuf_read_file(&buf, filename, 0) < 0) {
if (errno != ENOENT)
ret = error_errno(_("could not read '%s'", filename));
goto out;
}
if (starts_with(buf.buf,
"# This file was created and is maintained by git.\n") &&
unlink(filename))
ret = error_errno(_("could not remove '%s', filename));
out:
free(filename);
strbuf_release(&buf);
return ret;
}
Best Wishes
Phillip
-- >8 --
diff --git a/builtin/gc.c b/builtin/gc.c
index 99b158e481..7fb25ea2b1 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -2332,11 +2332,14 @@ static int systemd_set_units_state(int enable)
/*
* TODO: in the future (~2026 ?) remove this cleanup code
*/
+
static void systemd_delete_user_unit(char const *unit)
{
char const file_start_stale[] = "# This file was created and is"
" maintained by Git.";
- char file_start_user[sizeof(file_start_stale)] = {'\0'};
+ size_t const length = sizeof(file_start_stale);
+ char file_start_user[length] = {'\0'};
+
char *filename = xdg_config_home_for("systemd/user", unit);
int handle = open(filename, O_RDONLY);
@@ -2348,14 +2351,14 @@ static void systemd_delete_user_unit(char const *unit)
if (handle == -1 && !is_missing_file_error(errno))
warning(_("failed to delete '%s'"), filename);
else {
- read(handle, file_start_user, sizeof(file_start_stale) - 1);
- close(handle);
- if (strcmp(file_start_stale, file_start_user) == 0) {
+ if (length - 1 == read(handle, file_start_user, length - 1) &&
+ strcmp(file_start_stale, file_start_user) == 0) {
if (unlink(filename) == 0)
warning(_("deleted stale unit file '%s'"), filename);
else if (!is_missing_file_error(errno))
warning(_("failed to delete '%s'"), filename);
}
+ close(handle);
}
free(filename);
