Re: [Patch] can-utils/slcanpty.c pseudo-terminal interface

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 3061 bytes --]

Hello,

On 09/17/2012 12:12 AM, uescher wrote:
> Add Support for the Unix 98 pseudo-terminal interface /dev/ptmx /dev/pts/N

Cool, what kind of cool things can we do if this tool supports ptys?

> 
> please take a look over the little Patch.
> Thanks

Can you add a Signed-off-by[1] to your patch?

[1]
http://lxr.free-electrons.com/source/Documentation/SubmittingPatches#L298

Please don't use C++ comments, use Standard C comments. /* comment */.
Please remove any trailing whitespace from your patch.

Some nitpicking inline:

> 
> patch.diff
> 
> 
> diff --git a/slcanpty.c b/slcanpty.c
> index 67dcf35..e3b7304 100644
> --- a/slcanpty.c
> +++ b/slcanpty.c
> @@ -26,6 +26,11 @@
>   *
>   */
>  

I think it's okay to just define _GNU_SOURCE.

> +#ifndef _GNU_SOURCE  
> +// To get ptsname grantpt and unlockpt definitions from stdlib.h  
> +#define _GNU_SOURCE  
> +#endif  
> +
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> @@ -45,6 +50,8 @@
>  /* maximum rx buffer len: extended CAN frame with timestamp */
>  #define SLC_MTU (sizeof("T1111222281122334455667788EA5F\r")+1)
>  
> +#define DEVICE_NAME_PTMX "/dev/ptmx"
> +
>  #define DEBUG
>  
>  static int asc2nibble(char c)
> @@ -387,6 +394,8 @@ int main(int argc, char **argv)
>  		fprintf(stderr, "Usage: %s <pty> <can interface>\n", argv[0]);
>  		fprintf(stderr, "e.g. '%s /dev/ptyc0 can0' creates"
>  			" /dev/ttyc0 for the slcan application\n", argv[0]);
> +		fprintf(stderr, "e.g. for pseudo-terminal '%s /dev/ptmx can0' creates"
                                                              ^^^^^^^^^
Please use %s....

> +			" /dev/pts/N\n", argv[0]);
                                               ^^^
...and add ", DEVICE_NAME_PTMX" here.

>  		fprintf(stderr, "\n");
>  		return 1;
>  	}
> @@ -408,6 +417,27 @@ int main(int argc, char **argv)
>  			   ECHONL | ECHOPRT | ECHOKE | ICRNL);
>  	tcsetattr(p, TCSANOW, &topts);
>  
> +	//Support for the Unix 98 pseudo-terminal interface /dev/ptmx /dev/pts/N
> +	if  (strcmp(argv[1],DEVICE_NAME_PTMX) == 0) {
                          ^^^

Please add a space after the comma.

> +		if (grantpt(p) < 0) {
> +			perror("grantpt");
> +			return 1;
> +		}
> +
> +		if (unlockpt(p) < 0) {
> +			perror("unlockpt");
> +			return 1;
> +		}
> +
> +		char* name_pts = NULL;	/* slave pseudo-terminal device name */
                   ^^^
Please make it "char *name_pts".

> +		name_pts = ptsname(p);
> +		if (name_pts == NULL) {
> +			perror("ptsname");
> +			return 1;
> +		}
> +		printf("open: %s: slave pseudo-terminal is %s\n", argv[1], name_pts);
> +	}
> +
>  	/* open socket */
>  	s = socket(PF_CAN, SOCK_RAW, CAN_RAW);
>  	if (s < 0) {

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]