@matheusbsbs / matheus-bagliano / commits / 7cbbf9d6a2

write_entry: untangle symlink and regular-file cases

The write_entry() function switches on the mode of the entry we're going to write out. The cases for S_IFLNK and S_IFREG are lumped together. In earlier versions of the code, this made some sense. They have a shared preamble (which reads the blob content), a short type-specific body, and a shared conclusion (which writes out the file contents; always for S_IFREG and only sometimes for S_IFLNK). But over time this has grown to make less sense. The preamble now has conditional bits for each type, and the S_IFREG body has grown a lot more complicated. It's hard to follow the logic of which code is running for which mode. Let's give each mode its own case arm. We will still share the conclusion code, which means we now jump to it with a goto. Ideally we'd pull that shared code into its own function, but it touches so much internal state in the write_entry() function that the end result is actually harder to follow than the goto. While we're here, we'll touch up a few bits of whitespace to make the beginning and endings of the cases easier to read. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Oct 9, 2017 at 13:50 UTC 7cbbf9d6a275eb23b889158edb753d651542d5a9
1 file changed +40 -31
entry.c
+40 -31
@@ -260,13 +260,31 @@ static int write_entry(struct cache_entry *ce,
260 }
261
262 switch (ce_mode_s_ifmt) {
263 - case S_IFREG:
263 case S_IFLNK:
264 + new = read_blob_entry(ce, &size);
265 + if (!new)
266 + return error("unable to read sha1 file of %s (%s)",
267 + path, oid_to_hex(&ce->oid));
268 +
269 + /*
270 + * We can't make a real symlink; write out a regular file entry
271 + * with the symlink destination as its contents.
272 + */
273 + if (!has_symlinks || to_tempfile)
274 + goto write_file_entry;
275 +
276 + ret = symlink(new, path);
277 + free(new);
278 + if (ret)
279 + return error_errno("unable to create symlink %s", path);
280 + break;
281 +
282 + case S_IFREG:
283 /*
284 * We do not send the blob in case of a retry, so do not
285 * bother reading it at all.
286 */
269 - if (ce_mode_s_ifmt == S_IFREG && dco && dco->state == CE_RETRY) {
287 + if (dco && dco->state == CE_RETRY) {
288 new = NULL;
289 size = 0;
290 } else {
@@ -276,42 +294,31 @@ static int write_entry(struct cache_entry *ce,
294 path, oid_to_hex(&ce->oid));
295 }
296
279 - if (ce_mode_s_ifmt == S_IFLNK && has_symlinks && !to_tempfile) {
280 - ret = symlink(new, path);
281 - free(new);
282 - if (ret)
283 - return error_errno("unable to create symlink %s",
284 - path);
285 - break;
286 - }
287 -
297 /*
298 * Convert from git internal format to working tree format
299 */
291 - if (ce_mode_s_ifmt == S_IFREG) {
292 - if (dco && dco->state != CE_NO_DELAY) {
293 - ret = async_convert_to_working_tree(
294 - ce->name, new, size, &buf, dco);
295 - if (ret && string_list_has_string(&dco->paths, ce->name)) {
296 - free(new);
297 - goto delayed;
298 - }
299 - } else
300 - ret = convert_to_working_tree(
301 - ce->name, new, size, &buf);
302 -
303 - if (ret) {
300 + if (dco && dco->state != CE_NO_DELAY) {
301 + ret = async_convert_to_working_tree(ce->name, new,
302 + size, &buf, dco);
303 + if (ret && string_list_has_string(&dco->paths, ce->name)) {
304 free(new);
305 - new = strbuf_detach(&buf, &newsize);
306 - size = newsize;
305 + goto delayed;
306 }
308 - /*
309 - * No "else" here as errors from convert are OK at this
310 - * point. If the error would have been fatal (e.g.
311 - * filter is required), then we would have died already.
312 - */
307 + } else
308 + ret = convert_to_working_tree(ce->name, new, size, &buf);
309 +
310 + if (ret) {
311 + free(new);
312 + new = strbuf_detach(&buf, &newsize);
313 + size = newsize;
314 }
315 + /*
316 + * No "else" here as errors from convert are OK at this
317 + * point. If the error would have been fatal (e.g.
318 + * filter is required), then we would have died already.
319 + */
320
321 + write_file_entry:
322 fd = open_output_fd(path, ce, to_tempfile);
323 if (fd < 0) {
324 free(new);
@@ -326,6 +333,7 @@ static int write_entry(struct cache_entry *ce,
333 if (wrote != size)
334 return error("unable to write file %s", path);
335 break;
336 +
337 case S_IFGITLINK:
338 if (to_tempfile)
339 return error("cannot create temporary submodule %s", path);
@@ -337,6 +345,7 @@ static int write_entry(struct cache_entry *ce,
345 NULL, oid_to_hex(&ce->oid),
346 state->force ? SUBMODULE_MOVE_HEAD_FORCE : 0);
347 break;
348 +
349 default:
350 return error("unknown file mode for %s in index", path);
351 }