Re: [OpenWrt-Devel] [PATCH] maccalc: add option to read mac from temp file

2011-09-18 Thread Alexander Gordeev
В Fri, 16 Sep 2011 18:06:15 +0300
Roman Yeryomin leroi.li...@gmail.com пишет:

 On 16 September 2011 17:51, Alexander Gordeev lasa...@lvk.cs.msu.su wrote:
  В Fri, 16 Sep 2011 17:41:37 +0300
  Roman Yeryomin leroi.li...@gmail.com пишет:
 
  On 16 September 2011 01:40, Alexander Gordeev lasa...@lvk.cs.msu.su 
  wrote:
   В Fri, 26 Aug 2011 04:30:43 +0300
   Roman Yeryomin leroi.li...@gmail.com пишет:
  
   This method is much more stable than reading dd's output via stdin.
  
   What kind of problems do you have with piping dd's output to stdin?
  
 
  It outputs garbage very frequently and maccalc fails to convert the
  mac (and as a consequence uci-default script fails to set the real mac
  address). Try dd without piping to maccalc and you'll see.
  I've noticed this bug on ramips platform and can't say anything about
  other boards.
 
  Well, then this is probably a bug in dd? Or uClibc? Or kernel?
  Ok, I'll try to reproduce it. Can you please tell me what exactly is
  happening?
 
 
 Two different reads:
 root@OpenWrt:/# dd bs=1 skip=262148 count=6 if=/dev/mtd0
 u���6+0 records in
 6+0 records out
 root@OpenWrt:/# dd bs=1 skip=262148 count=6 if=/dev/mtd0
 u���6+0 records in
 6+0 records out
 
 I don't think this is the bug in dd/kernel/uclibc - it would probably
 expose when writing to a file too.
 Maybe it's something with busybox. I'm not sure of cause.

The bug is in maccalc. It does a single read() and expects to get
everything in one shot which doesn't happen sometimes. The patch is
attached. It fixes the problem according to my tests.

I don't know whether Roman's change is necessary anymore. What do you
think, Roman?

-- 
  Alexander
From ae35ee0ce9fc9b6cb5d8bea0a5a059df71602eab Mon Sep 17 00:00:00 2001
From: Alexander Gordeev lasa...@lvk.cs.msu.su
Date: Sun, 18 Sep 2011 21:43:12 +0400
Subject: [PATCH] maccalc: don't expect to get all data in one read

Signed-off-by: Alexander Gordeev lasa...@lvk.cs.msu.su
---
 package/maccalc/src/main.c |   31 ++-
 1 files changed, 30 insertions(+), 1 deletions(-)

diff --git a/package/maccalc/src/main.c b/package/maccalc/src/main.c
index e1e12cd..dcb5f55 100644
--- a/package/maccalc/src/main.c
+++ b/package/maccalc/src/main.c
@@ -9,6 +9,7 @@
  *
  */
 
+#include errno.h
 #include stdlib.h
 #include stdio.h
 #include stdint.h
@@ -124,6 +125,34 @@ static int maccalc_do_mac2bin(int argc, const char *argv[])
 	return 0;
 }
 
+static ssize_t read_safe(int fd, void *buf, size_t count)
+{
+	ssize_t total = 0;
+	ssize_t r;
+
+	while(count  0) {
+		r = read(fd, buf, count);
+		if (r == 0)
+			/* EOF */
+			break;
+		if (r  0) {
+			if (errno == EINTR)
+/* interrupted by a signal, restart */
+continue;
+			/* error */
+			total = -1;
+			break;
+		}
+
+		/* ok */
+		total += r;
+		count -= r;
+		buf += r;
+	}
+
+	return total;
+}
+
 static int maccalc_do_bin2mac(int argc, const char *argv[])
 {
 	unsigned char mac[MAC_ADDRESS_LEN];
@@ -134,7 +163,7 @@ static int maccalc_do_bin2mac(int argc, const char *argv[])
 		return ERR_INVALID;
 	}
 
-	c = read(STDIN_FILENO, mac, sizeof(mac));
+	c = read_safe(STDIN_FILENO, mac, sizeof(mac));
 	if (c != sizeof(mac)) {
 		fprintf(stderr, failed to read from stdin\n);
 		return ERR_IO;
-- 
1.7.5.4



signature.asc
Description: PGP signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH] maccalc: add option to read mac from temp file

2011-09-18 Thread Roman Yeryomin
On 18 September 2011 20:51, Alexander Gordeev lasa...@lvk.cs.msu.su wrote:
 В Fri, 16 Sep 2011 18:06:15 +0300
 Roman Yeryomin leroi.li...@gmail.com пишет:

 On 16 September 2011 17:51, Alexander Gordeev lasa...@lvk.cs.msu.su wrote:
  В Fri, 16 Sep 2011 17:41:37 +0300
  Roman Yeryomin leroi.li...@gmail.com пишет:
 
  On 16 September 2011 01:40, Alexander Gordeev lasa...@lvk.cs.msu.su 
  wrote:
   В Fri, 26 Aug 2011 04:30:43 +0300
   Roman Yeryomin leroi.li...@gmail.com пишет:
  
   This method is much more stable than reading dd's output via stdin.
  
   What kind of problems do you have with piping dd's output to stdin?
  
 
  It outputs garbage very frequently and maccalc fails to convert the
  mac (and as a consequence uci-default script fails to set the real mac
  address). Try dd without piping to maccalc and you'll see.
  I've noticed this bug on ramips platform and can't say anything about
  other boards.
 
  Well, then this is probably a bug in dd? Or uClibc? Or kernel?
  Ok, I'll try to reproduce it. Can you please tell me what exactly is
  happening?
 

 Two different reads:
 root@OpenWrt:/# dd bs=1 skip=262148 count=6 if=/dev/mtd0
 u���6+0 records in
 6+0 records out
 root@OpenWrt:/# dd bs=1 skip=262148 count=6 if=/dev/mtd0
 u���6+0 records in
 6+0 records out

 I don't think this is the bug in dd/kernel/uclibc - it would probably
 expose when writing to a file too.
 Maybe it's something with busybox. I'm not sure of cause.

 The bug is in maccalc. It does a single read() and expects to get
 everything in one shot which doesn't happen sometimes. The patch is
 attached. It fixes the problem according to my tests.

 I don't know whether Roman's change is necessary anymore. What do you
 think, Roman?


I think how it can be that you can't read 6 bytes from stdin in one
attempt but can do it from file?
It doesn't happen sometimes, it's about 50 to 70% of failed reads.
And it's not about the boards - I've tested on 3 completely different
ones.
I think that there is something to do with terminal translating binary
information into something unexpected. I forgot to mention that error
occurs when there are _more_ symbols in stdout (see above). Also note
that in both reads there are exactly 6 bytes reported by dd.

I'll test your patch.

Regards,
Roman
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH] maccalc: add option to read mac from temp file

2011-09-18 Thread Alexander Gordeev
В Sun, 18 Sep 2011 21:30:09 +0300
Roman Yeryomin leroi.li...@gmail.com пишет:

 On 18 September 2011 20:51, Alexander Gordeev lasa...@lvk.cs.msu.su wrote:
  В Fri, 16 Sep 2011 18:06:15 +0300
  Roman Yeryomin leroi.li...@gmail.com пишет:
 
  On 16 September 2011 17:51, Alexander Gordeev lasa...@lvk.cs.msu.su 
  wrote:
   В Fri, 16 Sep 2011 17:41:37 +0300
   Roman Yeryomin leroi.li...@gmail.com пишет:
  
   On 16 September 2011 01:40, Alexander Gordeev lasa...@lvk.cs.msu.su 
   wrote:
В Fri, 26 Aug 2011 04:30:43 +0300
Roman Yeryomin leroi.li...@gmail.com пишет:
   
This method is much more stable than reading dd's output via stdin.
   
What kind of problems do you have with piping dd's output to stdin?
   
  
   It outputs garbage very frequently and maccalc fails to convert the
   mac (and as a consequence uci-default script fails to set the real mac
   address). Try dd without piping to maccalc and you'll see.
   I've noticed this bug on ramips platform and can't say anything about
   other boards.
  
   Well, then this is probably a bug in dd? Or uClibc? Or kernel?
   Ok, I'll try to reproduce it. Can you please tell me what exactly is
   happening?
  
 
  Two different reads:
  root@OpenWrt:/# dd bs=1 skip=262148 count=6 if=/dev/mtd0
  u���6+0 records in
  6+0 records out
  root@OpenWrt:/# dd bs=1 skip=262148 count=6 if=/dev/mtd0
  u���6+0 records in
  6+0 records out
 
  I don't think this is the bug in dd/kernel/uclibc - it would probably
  expose when writing to a file too.
  Maybe it's something with busybox. I'm not sure of cause.
 
  The bug is in maccalc. It does a single read() and expects to get
  everything in one shot which doesn't happen sometimes. The patch is
  attached. It fixes the problem according to my tests.
 
  I don't know whether Roman's change is necessary anymore. What do you
  think, Roman?
 
 
 I think how it can be that you can't read 6 bytes from stdin in one
 attempt but can do it from file?

Quite easyly. BTW you shouldn't even expect to read() 6 bytes from file
at once.

For example:

$ strace -f -e trace=read,write sh -c 'dd if=/dev/urandom bs=1 count=6
| cat  /dev/null'
...
[pid  9661] read(0,  unfinished ...
...
[pid  9660] read(0, }, 1) = 1
[pid  9660] write(1, }, 1)= 1
[pid  9661] ... read resumed }, 32768) = 1
[pid  9661] write(1, }, 1)= 1
[pid  9660] read(0,  unfinished ...
[pid  9661] read(0,  unfinished ...
[pid  9660] ... read resumed \320, 1) = 1
[pid  9660] write(1, \320, 1) = 1
[pid  9661] ... read resumed \320, 32768) = 1
[pid  9660] read(0,  unfinished ...
[pid  9661] write(1, \320, 1) = 1
[pid  9660] ... read resumed x, 1)  = 1
[pid  9661] read(0,  unfinished ...
[pid  9660] write(1, x, 1)= 1
[pid  9661] ... read resumed x, 32768) = 1
[pid  9661] write(1, x, 1)= 1
[pid  9661] read(0,  unfinished ...
[pid  9660] read(0, \36, 1)   = 1
[pid  9660] write(1, \36, 1)  = 1
[pid  9661] ... read resumed \36, 32768) = 1
[pid  9661] write(1, \36, 1)  = 1
[pid  9660] read(0,  unfinished ...
[pid  9661] read(0,  unfinished ...
[pid  9660] ... read resumed \24, 1) = 1
[pid  9660] write(1, \24, 1)  = 1
[pid  9661] ... read resumed \24, 32768) = 1
[pid  9661] write(1, \24, 1)  = 1
[pid  9661] read(0,  unfinished ...
[pid  9660] read(0, \372, 1)  = 1
[pid  9660] write(1, \372, 1) = 1
[pid  9661] ... read resumed \372, 32768) = 1
[pid  9661] write(1, \372, 1) = 1
[pid  9661] read(0, , 32768)  = 0
...

cat tries to read 32768 but gets 1 byte on every read.
And, well, please read 'man 2 read'.

 It doesn't happen sometimes, it's about 50 to 70% of failed reads.
 And it's not about the boards - I've tested on 3 completely different
 ones.

Ok, it happens often. The more the value of the fix is.

 I think that there is something to do with terminal translating binary
 information into something unexpected. I forgot to mention that error
 occurs when there are _more_ symbols in stdout (see above). Also note
 that in both reads there are exactly 6 bytes reported by dd.
 
I think, you'd rather do no conclusions from binary output to a
console. I did that:

root@wimaxstore:/tmp# for i in 1 2 3 4 5 6 7 8 9 1 2 3 4 5 6 7 8 9; do dd bs=1 
skip=46 count=6 if=/dev/mtd2 2/dev/null | hexdump -C | head -n 1; done
  00 0c 43 30 52 66 |..C0Rf|
  00 0c 43 30 52 66 |..C0Rf|
  00 0c 43 30 52 66 |..C0Rf|
  00 0c 43 30 52 66 |..C0Rf|
  00 0c 43 30 52 66 |..C0Rf|
  00 0c 43 30 52 66 |..C0Rf|
  00 0c 43 30 52 66 |..C0Rf|
  00 0c 43 30 52 66 |..C0Rf|
  00 0c 43 30 52 66 |..C0Rf|

Re: [OpenWrt-Devel] [PATCH] maccalc: add option to read mac from temp file

2011-09-18 Thread Gert Doering
Hi,

On Sun, Sep 18, 2011 at 09:30:09PM +0300, Roman Yeryomin wrote:
 I think how it can be that you can't read 6 bytes from stdin in one
 attempt but can do it from file?

pipe semantics are different from file semantics.

A pipe read will return as soon as everything that has been written so
far has been read, and not wait for the reader buffer to fill up.

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


pgpUxBbCtfzzb.pgp
Description: PGP signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH] maccalc: add option to read mac from temp file

2011-09-18 Thread Gert Doering
Hi,

On Sun, Sep 18, 2011 at 10:58:23PM +0400, Alexander Gordeev wrote:
 BTW you shouldn't even expect to read() 6 bytes from file at once.

For normal files, read() will never read less than requested, unless you
hit an error or eof.

 $ strace -f -e trace=read,write sh -c 'dd if=/dev/urandom bs=1 count=6
 | cat  /dev/null'

/dev/urandom is not a *file*...

 cat tries to read 32768 but gets 1 byte on every read.

... and cat does not read from /dev/urandom in the first place here, but 
from the pipe from dd.

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


pgpgd6wzN41Vm.pgp
Description: PGP signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH] maccalc: add option to read mac from temp file

2011-09-16 Thread Roman Yeryomin
On 16 September 2011 01:40, Alexander Gordeev lasa...@lvk.cs.msu.su wrote:
 В Fri, 26 Aug 2011 04:30:43 +0300
 Roman Yeryomin leroi.li...@gmail.com пишет:

 This method is much more stable than reading dd's output via stdin.

 What kind of problems do you have with piping dd's output to stdin?


It outputs garbage very frequently and maccalc fails to convert the
mac (and as a consequence uci-default script fails to set the real mac
address). Try dd without piping to maccalc and you'll see.
I've noticed this bug on ramips platform and can't say anything about
other boards.

Regards,
Roman
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH] maccalc: add option to read mac from temp file

2011-09-16 Thread Alexander Gordeev
В Fri, 16 Sep 2011 17:41:37 +0300
Roman Yeryomin leroi.li...@gmail.com пишет:

 On 16 September 2011 01:40, Alexander Gordeev lasa...@lvk.cs.msu.su wrote:
  В Fri, 26 Aug 2011 04:30:43 +0300
  Roman Yeryomin leroi.li...@gmail.com пишет:
 
  This method is much more stable than reading dd's output via stdin.
 
  What kind of problems do you have with piping dd's output to stdin?
 
 
 It outputs garbage very frequently and maccalc fails to convert the
 mac (and as a consequence uci-default script fails to set the real mac
 address). Try dd without piping to maccalc and you'll see.
 I've noticed this bug on ramips platform and can't say anything about
 other boards.

Well, then this is probably a bug in dd? Or uClibc? Or kernel?
Ok, I'll try to reproduce it. Can you please tell me what exactly is
happening?

-- 
  Alexander


signature.asc
Description: PGP signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH] maccalc: add option to read mac from temp file

2011-09-16 Thread Florian Fainelli
On Friday 16 September 2011 16:51:16 Alexander Gordeev wrote:
 В Fri, 16 Sep 2011 17:41:37 +0300
 
 Roman Yeryomin leroi.li...@gmail.com пишет:
  On 16 September 2011 01:40, Alexander Gordeev lasa...@lvk.cs.msu.su 
wrote:
   В Fri, 26 Aug 2011 04:30:43 +0300
   
   Roman Yeryomin leroi.li...@gmail.com пишет:
   This method is much more stable than reading dd's output via stdin.
   
   What kind of problems do you have with piping dd's output to stdin?
  
  It outputs garbage very frequently and maccalc fails to convert the
  mac (and as a consequence uci-default script fails to set the real mac
  address). Try dd without piping to maccalc and you'll see.
  I've noticed this bug on ramips platform and can't say anything about
  other boards.
 
 Well, then this is probably a bug in dd? Or uClibc? Or kernel?
 Ok, I'll try to reproduce it. Can you please tell me what exactly is
 happening?

What happens if you insert some proper fflush() in maccalc?
-- 
Florian
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH] maccalc: add option to read mac from temp file

2011-09-16 Thread Roman Yeryomin
On 16 September 2011 17:51, Alexander Gordeev lasa...@lvk.cs.msu.su wrote:
 В Fri, 16 Sep 2011 17:41:37 +0300
 Roman Yeryomin leroi.li...@gmail.com пишет:

 On 16 September 2011 01:40, Alexander Gordeev lasa...@lvk.cs.msu.su wrote:
  В Fri, 26 Aug 2011 04:30:43 +0300
  Roman Yeryomin leroi.li...@gmail.com пишет:
 
  This method is much more stable than reading dd's output via stdin.
 
  What kind of problems do you have with piping dd's output to stdin?
 

 It outputs garbage very frequently and maccalc fails to convert the
 mac (and as a consequence uci-default script fails to set the real mac
 address). Try dd without piping to maccalc and you'll see.
 I've noticed this bug on ramips platform and can't say anything about
 other boards.

 Well, then this is probably a bug in dd? Or uClibc? Or kernel?
 Ok, I'll try to reproduce it. Can you please tell me what exactly is
 happening?


Two different reads:
root@OpenWrt:/# dd bs=1 skip=262148 count=6 if=/dev/mtd0
u���6+0 records in
6+0 records out
root@OpenWrt:/# dd bs=1 skip=262148 count=6 if=/dev/mtd0
u���6+0 records in
6+0 records out

I don't think this is the bug in dd/kernel/uclibc - it would probably
expose when writing to a file too.
Maybe it's something with busybox. I'm not sure of cause.

Regards,
Roman
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH] maccalc: add option to read mac from temp file

2011-09-15 Thread Alexander Gordeev
В Fri, 26 Aug 2011 04:30:43 +0300
Roman Yeryomin leroi.li...@gmail.com пишет:

 This method is much more stable than reading dd's output via stdin.

What kind of problems do you have with piping dd's output to stdin?

-- 
  Alexander


signature.asc
Description: PGP signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH] maccalc: add option to read mac from temp file

2011-09-12 Thread Roman Yeryomin
On 26 August 2011 04:30, Roman Yeryomin leroi.li...@gmail.com wrote:
 This method is much more stable than reading dd's output via stdin.

 Signed-off-by: Roman Yeryomin ro...@advem.lv

 Index: package/maccalc/src/main.c
 ===
 --- a/package/maccalc/src/main.c        (revision 28007)
 +++ b/package/maccalc/src/main.c        (working copy)


if there is nothing wrong, how about commiting this?

Regards,
Roman
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[OpenWrt-Devel] [PATCH] maccalc: add option to read mac from temp file

2011-08-25 Thread Roman Yeryomin
This method is much more stable than reading dd's output via stdin.

Signed-off-by: Roman Yeryomin ro...@advem.lv

Index: package/maccalc/src/main.c
===
--- a/package/maccalc/src/main.c(revision 28007)
+++ b/package/maccalc/src/main.c(working copy)
@@ -14,6 +14,7 @@
 #include stdint.h
 #include string.h
 #include unistd.h
+#include fcntl.h

 #define MAC_ADDRESS_LEN6

@@ -127,16 +128,27 @@
 static int maccalc_do_bin2mac(int argc, const char *argv[])
 {
unsigned char mac[MAC_ADDRESS_LEN];
+   int source;
ssize_t c;

-   if (argc != 0) {
-   usage();
-   return ERR_INVALID;
+   switch (argc) {
+   case 0:
+   source = STDIN_FILENO;
+   break;
+   case 1:
+   source = open(argv[0] , O_RDONLY);
+   break;
+   default:
+   usage();
+   return ERR_INVALID;
}

-   c = read(STDIN_FILENO, mac, sizeof(mac));
+   c = read(source, mac, sizeof(mac));
+   if (argc  0)
+   close(source);
+
if (c != sizeof(mac)) {
-   fprintf(stderr, failed to read from stdin\n);
+   fprintf(stderr, failed to read from %i\n, source);
return ERR_IO;
}

@@ -182,7 +194,7 @@
  add mac number\n
  and|or|xor mac1 mac2\n
  mac2bin mac\n
- bin2mac\n,
+ bin2mac [file]\n,
maccalc_name);
 }
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel