linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] candump: Add possibility to specify logfile
@ 2013-08-21  8:17 Alexander Stein
  2013-08-21  8:28 ` Marc Kleine-Budde
  0 siblings, 1 reply; 6+ messages in thread
From: Alexander Stein @ 2013-08-21  8:17 UTC (permalink / raw)
  To: linux-can; +Cc: Alexander Stein

You need to use -l/tmp/logfile (wihtout space!) because -l takes an
optional parameter.

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
 candump.c | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/candump.c b/candump.c
index c865bd7..72193fc 100644
--- a/candump.c
+++ b/candump.c
@@ -115,7 +115,7 @@ void print_usage(char *prg)
 	fprintf(stderr, "         -b <can>    (bridge mode - send received frames to <can>)\n");
 	fprintf(stderr, "         -B <can>    (bridge mode - like '-b' with disabled loopback)\n");
 	fprintf(stderr, "         -u <usecs>  (delay bridge forwarding by <usecs> microseconds)\n");
-	fprintf(stderr, "         -l          (log CAN-frames into file. Sets '-s %d' by default)\n", SILENT_ON);
+	fprintf(stderr, "         -l[logfile] (log CAN-frames into file. Sets '-s %d' by default)\n", SILENT_ON);
 	fprintf(stderr, "         -L          (use log file format on stdout)\n");
 	fprintf(stderr, "         -n <count>  (terminate after receiption of <count> CAN frames)\n");
 	fprintf(stderr, "         -r <size>   (set socket receive buffer to <size>)\n");
@@ -231,6 +231,7 @@ int main(int argc, char **argv)
 	struct timeval tv, last_tv;
 	struct timeval timeout, timeout_config = { 0, 0 }, *timeout_current = NULL;
 	FILE *logfile = NULL;
+	const char* fname = NULL;
 
 	signal(SIGTERM, sigterm);
 	signal(SIGHUP, sigterm);
@@ -239,7 +240,7 @@ int main(int argc, char **argv)
 	last_tv.tv_sec  = 0;
 	last_tv.tv_usec = 0;
 
-	while ((opt = getopt(argc, argv, "t:ciaSs:b:B:u:ldxLn:r:heT:?")) != -1) {
+	while ((opt = getopt(argc, argv, "t:ciaSs:b:B:u:l::dxLn:r:heT:?")) != -1) {
 		switch (opt) {
 		case 't':
 			timestamp = optarg[0];
@@ -324,6 +325,7 @@ int main(int argc, char **argv)
 
 		case 'l':
 			log = 1;
+			fname = optarg;
 			break;
 
 		case 'd':
@@ -566,22 +568,27 @@ int main(int argc, char **argv)
 	if (log) {
 		time_t currtime;
 		struct tm now;
-		char fname[sizeof("candump-2006-11-20_202026.log")+1];
+		char filename[sizeof("candump-2006-11-20_202026.log")+1];
 
-		if (time(&currtime) == (time_t)-1) {
-			perror("time");
-			return 1;
-		}
+		if (fname == NULL)
+		{
+			if (time(&currtime) == (time_t)-1) {
+				perror("time");
+				return 1;
+			}
+
+			localtime_r(&currtime, &now);
 
-		localtime_r(&currtime, &now);
+			sprintf(filename, "candump-%04d-%02d-%02d_%02d%02d%02d.log",
+				now.tm_year + 1900,
+				now.tm_mon + 1,
+				now.tm_mday,
+				now.tm_hour,
+				now.tm_min,
+				now.tm_sec);
 
-		sprintf(fname, "candump-%04d-%02d-%02d_%02d%02d%02d.log",
-			now.tm_year + 1900,
-			now.tm_mon + 1,
-			now.tm_mday,
-			now.tm_hour,
-			now.tm_min,
-			now.tm_sec);
+			fname = filename;
+		}
 
 		if (silent != SILENT_ON)
 			printf("\nWarning: console output active while logging!");
-- 
1.8.1.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] candump: Add possibility to specify logfile
  2013-08-21  8:17 [PATCH] candump: Add possibility to specify logfile Alexander Stein
@ 2013-08-21  8:28 ` Marc Kleine-Budde
  2013-08-21  8:29   ` Marc Kleine-Budde
  0 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2013-08-21  8:28 UTC (permalink / raw)
  To: Alexander Stein; +Cc: linux-can

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

On 08/21/2013 10:17 AM, Alexander Stein wrote:
> You need to use -l/tmp/logfile (wihtout space!) because -l takes an
> optional parameter.
> 
> Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>

Please use the Kernel Coding Style.

Marc

> ---
>  candump.c | 37 ++++++++++++++++++++++---------------
>  1 file changed, 22 insertions(+), 15 deletions(-)
> 
> diff --git a/candump.c b/candump.c
> index c865bd7..72193fc 100644
> --- a/candump.c
> +++ b/candump.c
> @@ -115,7 +115,7 @@ void print_usage(char *prg)
>  	fprintf(stderr, "         -b <can>    (bridge mode - send received frames to <can>)\n");
>  	fprintf(stderr, "         -B <can>    (bridge mode - like '-b' with disabled loopback)\n");
>  	fprintf(stderr, "         -u <usecs>  (delay bridge forwarding by <usecs> microseconds)\n");
> -	fprintf(stderr, "         -l          (log CAN-frames into file. Sets '-s %d' by default)\n", SILENT_ON);
> +	fprintf(stderr, "         -l[logfile] (log CAN-frames into file. Sets '-s %d' by default)\n", SILENT_ON);
>  	fprintf(stderr, "         -L          (use log file format on stdout)\n");
>  	fprintf(stderr, "         -n <count>  (terminate after receiption of <count> CAN frames)\n");
>  	fprintf(stderr, "         -r <size>   (set socket receive buffer to <size>)\n");
> @@ -231,6 +231,7 @@ int main(int argc, char **argv)
>  	struct timeval tv, last_tv;
>  	struct timeval timeout, timeout_config = { 0, 0 }, *timeout_current = NULL;
>  	FILE *logfile = NULL;
> +	const char* fname = NULL;

chat *fname

>  
>  	signal(SIGTERM, sigterm);
>  	signal(SIGHUP, sigterm);
> @@ -239,7 +240,7 @@ int main(int argc, char **argv)
>  	last_tv.tv_sec  = 0;
>  	last_tv.tv_usec = 0;
>  
> -	while ((opt = getopt(argc, argv, "t:ciaSs:b:B:u:ldxLn:r:heT:?")) != -1) {
> +	while ((opt = getopt(argc, argv, "t:ciaSs:b:B:u:l::dxLn:r:heT:?")) != -1) {
>  		switch (opt) {
>  		case 't':
>  			timestamp = optarg[0];
> @@ -324,6 +325,7 @@ int main(int argc, char **argv)
>  
>  		case 'l':
>  			log = 1;
> +			fname = optarg;
>  			break;
>  
>  		case 'd':
> @@ -566,22 +568,27 @@ int main(int argc, char **argv)
>  	if (log) {
>  		time_t currtime;
>  		struct tm now;
> -		char fname[sizeof("candump-2006-11-20_202026.log")+1];
> +		char filename[sizeof("candump-2006-11-20_202026.log")+1];
                                                                    ^^^^

Space between  ) and +
>  
> -		if (time(&currtime) == (time_t)-1) {
> -			perror("time");
> -			return 1;
> -		}
> +		if (fname == NULL)
> +		{

if () {

> +			if (time(&currtime) == (time_t)-1) {
> +				perror("time");
> +				return 1;
> +			}
> +
> +			localtime_r(&currtime, &now);
>  
> -		localtime_r(&currtime, &now);
> +			sprintf(filename, "candump-%04d-%02d-%02d_%02d%02d%02d.log",
> +				now.tm_year + 1900,
> +				now.tm_mon + 1,
> +				now.tm_mday,
> +				now.tm_hour,
> +				now.tm_min,
> +				now.tm_sec);
>  
> -		sprintf(fname, "candump-%04d-%02d-%02d_%02d%02d%02d.log",
> -			now.tm_year + 1900,
> -			now.tm_mon + 1,
> -			now.tm_mday,
> -			now.tm_hour,
> -			now.tm_min,
> -			now.tm_sec);
> +			fname = filename;
> +		}
>  
>  		if (silent != SILENT_ON)
>  			printf("\nWarning: console output active while logging!");
> 


-- 
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 --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] candump: Add possibility to specify logfile
  2013-08-21  8:28 ` Marc Kleine-Budde
@ 2013-08-21  8:29   ` Marc Kleine-Budde
  2013-08-21  9:21     ` [PATCH v2] " Alexander Stein
  0 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2013-08-21  8:29 UTC (permalink / raw)
  To: Alexander Stein; +Cc: linux-can

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

On 08/21/2013 10:28 AM, Marc Kleine-Budde wrote:
>> +	const char* fname = NULL;
> 
> chat *fname
char

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 --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] candump: Add possibility to specify logfile
  2013-08-21  8:29   ` Marc Kleine-Budde
@ 2013-08-21  9:21     ` Alexander Stein
  2013-08-21 16:58       ` Oliver Hartkopp
  0 siblings, 1 reply; 6+ messages in thread
From: Alexander Stein @ 2013-08-21  9:21 UTC (permalink / raw)
  To: linux-can; +Cc: Alexander Stein

You need to use -l/tmp/logfile (wihtout space!) because -l takes an
optional parameter.

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
Changes in v2:
* Fixed some coding style issues

 candump.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/candump.c b/candump.c
index c865bd7..35ccaad 100644
--- a/candump.c
+++ b/candump.c
@@ -115,7 +115,7 @@ void print_usage(char *prg)
 	fprintf(stderr, "         -b <can>    (bridge mode - send received frames to <can>)\n");
 	fprintf(stderr, "         -B <can>    (bridge mode - like '-b' with disabled loopback)\n");
 	fprintf(stderr, "         -u <usecs>  (delay bridge forwarding by <usecs> microseconds)\n");
-	fprintf(stderr, "         -l          (log CAN-frames into file. Sets '-s %d' by default)\n", SILENT_ON);
+	fprintf(stderr, "         -l[logfile] (log CAN-frames into file. Sets '-s %d' by default)\n", SILENT_ON);
 	fprintf(stderr, "         -L          (use log file format on stdout)\n");
 	fprintf(stderr, "         -n <count>  (terminate after receiption of <count> CAN frames)\n");
 	fprintf(stderr, "         -r <size>   (set socket receive buffer to <size>)\n");
@@ -231,6 +231,7 @@ int main(int argc, char **argv)
 	struct timeval tv, last_tv;
 	struct timeval timeout, timeout_config = { 0, 0 }, *timeout_current = NULL;
 	FILE *logfile = NULL;
+	const char *fname = NULL;
 
 	signal(SIGTERM, sigterm);
 	signal(SIGHUP, sigterm);
@@ -239,7 +240,7 @@ int main(int argc, char **argv)
 	last_tv.tv_sec  = 0;
 	last_tv.tv_usec = 0;
 
-	while ((opt = getopt(argc, argv, "t:ciaSs:b:B:u:ldxLn:r:heT:?")) != -1) {
+	while ((opt = getopt(argc, argv, "t:ciaSs:b:B:u:l::dxLn:r:heT:?")) != -1) {
 		switch (opt) {
 		case 't':
 			timestamp = optarg[0];
@@ -324,6 +325,7 @@ int main(int argc, char **argv)
 
 		case 'l':
 			log = 1;
+			fname = optarg;
 			break;
 
 		case 'd':
@@ -566,22 +568,26 @@ int main(int argc, char **argv)
 	if (log) {
 		time_t currtime;
 		struct tm now;
-		char fname[sizeof("candump-2006-11-20_202026.log")+1];
+		char filename[sizeof("candump-2006-11-20_202026.log") + 1];
 
-		if (time(&currtime) == (time_t)-1) {
-			perror("time");
-			return 1;
-		}
+		if (fname == NULL) {
+			if (time(&currtime) == (time_t)-1) {
+				perror("time");
+				return 1;
+			}
+
+			localtime_r(&currtime, &now);
 
-		localtime_r(&currtime, &now);
+			sprintf(filename, "candump-%04d-%02d-%02d_%02d%02d%02d.log",
+				now.tm_year + 1900,
+				now.tm_mon + 1,
+				now.tm_mday,
+				now.tm_hour,
+				now.tm_min,
+				now.tm_sec);
 
-		sprintf(fname, "candump-%04d-%02d-%02d_%02d%02d%02d.log",
-			now.tm_year + 1900,
-			now.tm_mon + 1,
-			now.tm_mday,
-			now.tm_hour,
-			now.tm_min,
-			now.tm_sec);
+			fname = filename;
+		}
 
 		if (silent != SILENT_ON)
 			printf("\nWarning: console output active while logging!");
-- 
1.8.1.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] candump: Add possibility to specify logfile
  2013-08-21  9:21     ` [PATCH v2] " Alexander Stein
@ 2013-08-21 16:58       ` Oliver Hartkopp
  2013-08-22  6:04         ` Alexander Stein
  0 siblings, 1 reply; 6+ messages in thread
From: Oliver Hartkopp @ 2013-08-21 16:58 UTC (permalink / raw)
  To: Alexander Stein; +Cc: linux-can

On 21.08.2013 11:21, Alexander Stein wrote:
> You need to use -l/tmp/logfile (wihtout space!) because -l takes an
> optional parameter.
> 

Hm. I don't like that because of the "(without space!)"

If the option could have been passed like all other options (with space), it
would have been ok. But not like this.

I know that this is a getopt problem ...

Btw. why can't you just use '-L' and redirect the output, e.g.

candump -L can0 > mylogfile.log

??

Regards,
Oliver



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] candump: Add possibility to specify logfile
  2013-08-21 16:58       ` Oliver Hartkopp
@ 2013-08-22  6:04         ` Alexander Stein
  0 siblings, 0 replies; 6+ messages in thread
From: Alexander Stein @ 2013-08-22  6:04 UTC (permalink / raw)
  To: Oliver Hartkopp; +Cc: linux-can

Hello Oliver,

On Wednesday 21 August 2013 18:58:19, Oliver Hartkopp wrote:
> On 21.08.2013 11:21, Alexander Stein wrote:
> > You need to use -l/tmp/logfile (wihtout space!) because -l takes an
> > optional parameter.
> > 
> 
> Hm. I don't like that because of the "(without space!)"
> 
> If the option could have been passed like all other options (with space), it
> would have been ok. But not like this.
> 
> I know that this is a getopt problem ...

Exactly. I was wondering my self, but otherwise how can an optional parameter implemented in a different way?

> Btw. why can't you just use '-L' and redirect the output, e.g.
> 
> candump -L can0 > mylogfile.log

You can, certainly. I was just wondering why the user must not specify a logfile name at first place.

Regards,
Alexander


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-08-22  6:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-21  8:17 [PATCH] candump: Add possibility to specify logfile Alexander Stein
2013-08-21  8:28 ` Marc Kleine-Budde
2013-08-21  8:29   ` Marc Kleine-Budde
2013-08-21  9:21     ` [PATCH v2] " Alexander Stein
2013-08-21 16:58       ` Oliver Hartkopp
2013-08-22  6:04         ` Alexander Stein

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).