introduce strbuf_read_cmd to avoid deadlocks
Get series with b4
b4 am [email protected]Versions newest first
- v1 Latest7 patches 6 repliesIn review
Patches in v1 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/7 | wt-status: don't flush before running "submodule status"wt-status.c | +0 | 0 replies | |
| 2/7 | wt_status: fix signedness mismatch in strbuf_read callwt-status.c | +3 | 0 replies | |
| 3/7 | strbuf: introduce strbuf_read_cmd helperstrbuf.c strbuf.h | +27 | 15 replies | |
| 4/7 | wt-status: use strbuf_read_cmdwt-status.c | +1 | 0 replies | |
| 5/7 | submodule: use strbuf_read_cmdsubmodule.c | +1 | 0 replies | |
| 6/7 | trailer: use strbuf_read_cmdtrailer.c | +5 | 0 replies | |
| 7/7 | run-command: forbid using run_command with piped outputrun-command.c | +6 | 0 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.
Cover letter [PATCH 0/7] introduce strbuf_read_cmd to avoid deadlocks
wt-status.c static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt · line 753
Quoted 13 lines · Jeff King
wt-status.c +2
-2@@ -748,9 +748,9 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommittfflush (s->fp); sm_summary.out =-1 ; -run_command (&sm_summary); - +start_command (&sm_summary); len =strbuf_read (&cmd_stdout, sm_summary.out,1024 ); +finish_command (&sm_summary);
This isn't quite right. In both the original and the new one, if start_command fails, we may call strbuf_read on whatever it happens to have left in sm_summary.out. Worse, in the new one, we would call finish_command on something that was never started. And in both old and new, we leak the sm_summary.out descriptor.
Furthermore, I found two other potential deadlocks (and one more descriptor leak). I tried at first to fix them all up like I did with the above patch, but I found that I was introducing similar problems in each case. So instead, I pulled this pattern out into its own strbuf helper.
So here's a replacement series.
[1/7]: wt-status: don't flush before running "submodule status" [2/7]: wt_status: fix signedness mismatch in strbuf_read call [3/7]: strbuf: introduce strbuf_read_cmd helper [4/7]: wt-status: use strbuf_read_cmd
[5/7]: submodule: use strbuf_read_cmd
[6/7]: trailer: use strbuf_read_cmd
[7/7]: run-command: forbid using run_command with piped output
-Peff