On Wed, Dec 22, 2021 at 8:40 AM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:

On Tue, Dec 21 2021, Han Xin wrote:

quoted

From: Han Xin <redacted>
[...]

@@ -1854,17 +1876,48 @@ static int create_tmpfile(struct strbuf *tmp, const char *filename)
              strbuf_reset(tmp);
              strbuf_add(tmp, filename, dirlen - 1);
              if (mkdir(tmp->buf, 0777) && errno != EEXIST)
-                     return -1;
+                     break;
              if (adjust_shared_perm(tmp->buf))
-                     return -1;
+                     break;

              /* Try again */
              strbuf_addstr(tmp, "/tmp_obj_XXXXXX");
              fd = git_mkstemp_mode(tmp->buf, 0444);
+     } while (0);
+
+     if (fd < 0 && !(flags & HASH_SILENT)) {
+             if (errno == EACCES)
+                     return error(_("insufficient permission for adding an "
+                                    "object to repository database %s"),
+                                  get_object_directory());

This should be an error_errno() instead, ...

We already know the errno (EACCESS) and output a decent error message,
so using error() is OK.  BTW, it's just a refactor by copy & paste.

quoted

+             else
+                     return error_errno(_("unable to create temporary file"));

...and we can just fold this whole if/else into one condition with a
briefer message, e.g.:

    error_errno(_("unable to add object to '%s'"), get_object_directory());

Or whatever, unless there's another bug here where you inverted these
conditions, and the "else" really should not use "error_errno" but
"error".... (I don't know...)