nsxiv658a935
fix: potentially printing wrong error message (#321)
it's possible for the close() calls to override the errno resulting in incorrect error message being printed. call error() immediately to avoid such possibilities. also refactor a couple conditions to avoid doing multiple checks.
+17 −22
2 files changed
| @@ -567,8 +567,8 @@ static bool run_key_handler(const char *key, unsigned int mask) | |||
| 567 | 567 | if (pfd.writefd < 0) | |
| 568 | 568 | return false; | |
| 569 | 569 | if ((pfs = fdopen(pfd.writefd, "w")) == NULL) { | |
| 570 | − | close(pfd.writefd); | |
| 571 | 570 | error(0, errno, "open pipe"); | |
| 571 | + | close(pfd.writefd); | |
| 572 | 572 | return false; | |
| 573 | 573 | } | |
| 574 | 574 | ||
| @@ -233,8 +233,7 @@ spawn_t spawn(const char *cmd, char *const argv[], unsigned int flags) | |||
| 233 | 233 | { | |
| 234 | 234 | pid_t pid; | |
| 235 | 235 | spawn_t status = { -1, -1, -1 }; | |
| 236 | − | int pfd_read[2] = { -1, -1 }; | |
| 237 | − | int pfd_write[2] = { -1, -1 }; | |
| 236 | + | int pfd_read[2] = { -1, -1 }, pfd_write[2] = { -1, -1 }; | |
| 238 | 237 | const bool r = flags & X_READ; | |
| 239 | 238 | const bool w = flags & X_WRITE; | |
| 240 | 239 | ||
| @@ -247,16 +246,18 @@ spawn_t spawn(const char *cmd, char *const argv[], unsigned int flags) | |||
| 247 | 246 | } | |
| 248 | 247 | ||
| 249 | 248 | if (w && pipe(pfd_write) < 0) { | |
| 249 | + | error(0, errno, "pipe: %s", cmd); | |
| 250 | 250 | if (r) { | |
| 251 | 251 | close(pfd_read[0]); | |
| 252 | 252 | close(pfd_read[1]); | |
| 253 | 253 | } | |
| 254 | − | error(0, errno, "pipe: %s", cmd); | |
| 255 | 254 | return status; | |
| 256 | 255 | } | |
| 257 | 256 | ||
| 258 | − | if ((pid = fork()) == 0) { | |
| 259 | − | bool err = (r && dup2(pfd_read[1], 1) < 0) || (w && dup2(pfd_write[0], 0) < 0); | |
| 257 | + | if ((pid = fork()) == 0) { /* in child */ | |
| 258 | + | if ((r && dup2(pfd_read[1], 1) < 0) || (w && dup2(pfd_write[0], 0) < 0)) | |
| 259 | + | error(EXIT_FAILURE, errno, "dup2: %s", cmd); | |
| 260 | + | ||
| 260 | 261 | if (r) { | |
| 261 | 262 | close(pfd_read[0]); | |
| 262 | 263 | close(pfd_read[1]); | |
| @@ -265,29 +266,23 @@ spawn_t spawn(const char *cmd, char *const argv[], unsigned int flags) | |||
| 265 | 266 | close(pfd_write[0]); | |
| 266 | 267 | close(pfd_write[1]); | |
| 267 | 268 | } | |
| 268 | − | ||
| 269 | − | if (err) | |
| 270 | − | error(EXIT_FAILURE, errno, "dup2: %s", cmd); | |
| 271 | 269 | execv(cmd, argv); | |
| 272 | 270 | error(EXIT_FAILURE, errno, "exec: %s", cmd); | |
| 273 | − | } | |
| 274 | − | ||
| 275 | − | if (r) | |
| 276 | − | close(pfd_read[1]); | |
| 277 | − | if (w) | |
| 278 | − | close(pfd_write[0]); | |
| 279 | − | ||
| 280 | − | if (pid < 0) { | |
| 271 | + | } else if (pid < 0) { /* fork failed */ | |
| 272 | + | error(0, errno, "fork: %s", cmd); | |
| 281 | 273 | if (r) | |
| 282 | 274 | close(pfd_read[0]); | |
| 283 | 275 | if (w) | |
| 284 | 276 | close(pfd_write[1]); | |
| 285 | − | error(0, errno, "fork: %s", cmd); | |
| 286 | − | return status; | |
| 277 | + | } else { /* in parent */ | |
| 278 | + | status.pid = pid; | |
| 279 | + | status.readfd = pfd_read[0]; | |
| 280 | + | status.writefd = pfd_write[1]; | |
| 287 | 281 | } | |
| 288 | 282 | ||
| 289 | − | status.pid = pid; | |
| 290 | − | status.readfd = pfd_read[0]; | |
| 291 | − | status.writefd = pfd_write[1]; | |
| 283 | + | if (r) | |
| 284 | + | close(pfd_read[1]); | |
| 285 | + | if (w) | |
| 286 | + | close(pfd_write[0]); | |
| 292 | 287 | return status; | |
| 293 | 288 | } | |