On Sun, Mar 24, 2024 at 03:45:45PM +0000, Phillip Wood wrote:
> 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.
In that case, should we hook the cleanup (in it's final form) in more
place ? `git maintenance register` for instance ?
>
> > > +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;
> }
Thanks, this is exactly what I needed, I looked at the strbuf API but
couldn't find this somehow.
--
Max Gautier
