[PATCH v2 1/2] Use a shell command as a socket for gdbserver
Mike Frysinger
vapier@gentoo.org
Tue May 5 02:58:00 GMT 2015
On 04 May 2015 23:54, Gabriel Corona wrote:
> +static int
> +open_shell_command (char *command)
const ?
> +{
> + int sockets[2];
> + pid_t child, grandchild, res;
> + int status;
> +
> + if (socketpair (AF_LOCAL, SOCK_STREAM, 0, sockets) < 0)
> + perror_with_name ("Can't get socketpair");
> + child = fork ();
prefer a blank line above the fork assignment
> + if (dup2 (sockets[1], 0) < 0 || dup2 (sockets[1], 1) < 0)
> + perror_with_name ("Can't dup socket to stdio");
use STDIN_FILENO & STDOUT_FILENO instead of 0 & 1
> + if (close (sockets[1]) < 0)
> + perror_with_name ("Can't close original socket");
dup2 has an edge case where if the fd is the same as an existing one, it won't
create a new one. i think before you close, you need to compare it to
STDIN_FILENO & STDOUT_FILENO and only close it if it doesn't match.
> + /* Double fork in order to inherit the grandchild. The process
> + is expected to exit when the other end of the socketpair is
> + closed.
> + */
prefer a blank line above this comment to space the code out. also, GNU style
says:
- use two spaces after periods
- cuddle the trailing */ rather than putting it on a new line by itself
> + else
> + {
> + close (sockets[1]);
check return value ?
> + signal (SIGPIPE, SIG_IGN);
> + while ((res = waitpid (child, &status, 0)) < 0 && errno == EINTR);
> + if (res < 0)
> + perror_with_name ("Can't wait child process");
phrasing is awkward ... maybe insert "on" or "for the" in there ?
> + }
> + return -1;
> +}
i think GNU style says you should have a blank line above this last return
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20150505/a3bbde3d/attachment.sig>
More information about the Gdb-patches
mailing list