From: Junio C Hamano Date: Thu, 18 Dec 2025 00:32:57 GMT Subject: Re: [PATCH v2] lockfile: add PID file for debugging stale locks Message-ID: In-Reply-To: "Paulo Casaretto via GitGitGadget" writes: > /* Make sure errno contains a meaningful value on error */ > static int lock_file(struct lock_file *lk, const char *path, int flags, > - int mode) > + int mode, enum lockfile_pid_component component) > ... > } > > @@ -102,7 +203,8 @@ static int lock_file(struct lock_file *lk, const char *path, int flags, > * exactly once. If timeout_ms is -1, try indefinitely. > */ > static int lock_file_timeout(struct lock_file *lk, const char *path, > - int flags, long timeout_ms, int mode) > + int flags, long timeout_ms, int mode, > + enum lockfile_pid_component component) > { > ... > if (timeout_ms == 0) > - return lock_file(lk, path, flags, mode); > + return lock_file(lk, path, flags, mode, component); > - fd = lock_file(lk, path, flags, mode); > + fd = lock_file(lk, path, flags, mode, component); These are OK, but I expected these are rolled into an "unsigned flags" word, so that ... > int hold_lock_file_for_update_timeout_mode( > - struct lock_file *lk, const char *path, > - int flags, long timeout_ms, int mode); > + struct lock_file *lk, const char *path, > + int flags, long timeout_ms, int mode, > + enum lockfile_pid_component component); ... things like this can be done without adding an extra parameter. Compared to "what should we do when we see an error?", ... > - fd = hold_lock_file_for_update_timeout(&lock, path.buf, LOCK_DIE_ON_ERROR, -1); > + fd = hold_lock_file_for_update_timeout(&lock, path.buf, LOCK_DIE_ON_ERROR, -1, > + LOCKFILE_PID_OTHER); ... "how would we name the lockfile for this action?" is *not* all that special and should not occupy a separate parameter on its own. Existing "flags" argument being "int" not "unsigned int" is a historical mistake, by the way. But maybe it is just me? I dunno.