Re: [RFC][PATCH][cryo] Save/restore state of unnamed pipes

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org]

Matt Helsley [matthltc-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org] wrote:
| 
| Yes, I think that's sufficient:
| 
|         int pipefds[2];
| 
| 	...
| 
| 	restarted_read_fd = 11;
| 	restarted_write_fd = 12;
| 
| 	...
| 
|         pipe(pipefds);
| 
| 	/* 
| 	 * pipe() may have returned one (or both) of the restarted fds
| 	 * at the wrong end of the pipe. This could cause dup2() to
| 	 * accidentaly close the pipe. Avoid that with an extra dup().
| 	 */
|         if (pipefds[1] == restarted_read_fd) {
| 		dup2(pipefds[1], last_fd + 1);
| 		pipefds[1] = last_fd + 1;
| 	}
| 
| 	if (pipefds[0] != restarted_read_fd) {
| 		dup2(pipefds[0], restarted_read_fd);
| 		close(pipefds[0]);
| 	}
| 
| 	if (pipefds[0] != restarted_read_fd) {
| 	        dup2(pipefds[1], restarted_write_fd);
| 		close(pipefds[1]);
| 	}

Shouldn't the last if be

	if (pipefds[1] != restarted_wrte_fd) ?

(otherwise it would break if pipefds[0] = 11 and pipefds[1] = 200)

I came up with something similar, but with an extra close(). And
in my code, I had restarted_* names referring to pipefds[] making
it a bit confusing initially.

How about using actual_fds[] (instead of pipefds) and expected_fds[]
instead of (restart_*) ? 

Thanks,

Suka

| 
| I think this code does the minimal number of operations needed in the
| restarted application too -- it counts on the second dup2() closing one
| of the fds if pipefds[1] == restarted_read_fd.
| 
| Cheers,
| 	-Matt