adding a new log-format escape

2005-06-27 Thread Andrew Shewmaker
I'm adding a new escape to log-format, %s, to print out the checksum
of a file, and I've got  a couple problems.  They've got to be simple
bugs, but I haven't been able to figure them out.  The following patch
gives me a broken pipe and a bus error when I test it.  Note that I've
applied the md5 patch beforehand.

diff -Naur rsync-2.6.5-md5/log.c rsync-2.6.5/log.c
--- rsync-2.6.5-md5/log.c   2005-06-26 22:34:17.0 -0600
+++ rsync-2.6.5/log.c   2005-06-26 22:38:57.0 -0600
@@ -371,7 +371,7 @@
char buf[MAXPATHLEN+1024], buf2[MAXPATHLEN], fmt[32];
char *p, *s, *n;
size_t len, total;
-   int64 b;
+   int64 b, j;
 
*fmt = '%';
 
@@ -483,6 +483,13 @@
snprintf(buf2, sizeof buf2, fmt, (double)b);
n = buf2;
break;
+   case 's':
+   strlcat(fmt, 02x, sizeof fmt);
+   for (j = 0; j  SUM_LENGTH; j++ ) {
+   snprintf(buf2 + j * 2, sizeof buf2,
fmt, file-u.sum[j]);
+   }
+   n = buf2;
+   break;
case 'i':
if (iflags  ITEM_DELETED) {
n = *deleting;

$ rsync -acv --md5 --log-format=%c %s  %f rsync-2.6.5 blah
building file list ... done
created directory blah
rsync: writefd_unbuffered failed to write 82 bytes: phase unknown
[generator]: Broken pipe (32)
rsync error: error in rsync protocol data stream (code 12) at io.c(1099)
Bus error

I get the same error if I replace SUM_LENGTH with a literal 16. 
Interestingly, when I reuse the code to calculate the number of
checksum bytes received for the file, I don't get an error.

diff -Naur rsync-2.6.5-md5/log.c rsync-2.6.5/log.c
--- rsync-2.6.5-md5/log.c   2005-06-26 22:34:17.0 -0600
+++ rsync-2.6.5/log.c   2005-06-26 22:17:12.0 -0600
@@ -371,7 +371,7 @@
char buf[MAXPATHLEN+1024], buf2[MAXPATHLEN], fmt[32];
char *p, *s, *n;
size_t len, total;
-   int64 b;
+   int64 b, j;
 
*fmt = '%';
 
@@ -483,6 +483,20 @@
snprintf(buf2, sizeof buf2, fmt, (double)b);
n = buf2;
break;
+   case 's':
+   if (!am_sender) {
+   b = stats.total_written -
+   initial_stats-total_written;
+   } else {
+   b = stats.total_read -
+   initial_stats-total_read;
+   }
+   strlcat(fmt, 02x, sizeof fmt);
+   for (j = 0; j  b; j++ ) {
+   snprintf(buf2 + j * 2, sizeof buf2,
fmt, file-u.sum[j]);
+   }
+   n = buf2;
+   break;
case 'i':
if (iflags  ITEM_DELETED) {
n = *deleting;

$ rsync -acv --md5 --log-format=%c %s  %f rsync-2.6.5 blah
building file list ... done
created directory blah
0 0  rsync-2.6.5
16 5709ff6fff662aff2968302f  rsync-2.6.5/.ignore
16 393a5cff45587325ff17ff33  rsync-2.6.5/COPYING
16 ff3d3322ff7aff77  rsync-2.6.5/Doxyfile
16 ff241c784fff274638ff4eff96  rsync-2.6.5/INSTALL
16 ff013e6c15ff700b  rsync-2.6.5/Makefile
16 63604d4321ff183622fff4  rsync-2.6.5/Makefile.in
...

As you can see the number of checksum bytes is always 16 for files and
I don't get any errors like I do above.  However, the checksums are
wrong (I can see short segments that are right).  They don't match the
output from md5sum nor the test program in md5.c, which prints the hex
string in nearly the same way.  Also, some of the hex strings are
longer than others even though they have the same number of checksum
bytes.

Any ideas what I'm doing wrong?

BTW, this behaves the same on OS X Tiger w/ GCC 4.0 and Fedora Core 1 x86.

Thanks in advance for any help,


Andrew Shewmaker
--
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html


Re: adding a new log-format escape

2005-06-27 Thread YOSHIFUJI Hideaki / 吉藤英明
In article [EMAIL PROTECTED] (at Mon, 27 Jun 2005 00:07:19 -0600), Andrew 
Shewmaker [EMAIL PROTECTED] says:

 +   for (j = 0; j  SUM_LENGTH; j++ ) {
 +   snprintf(buf2 + j * 2, sizeof buf2,
 fmt, file-u.sum[j]);

   file-u.sum[j]  0xff

 +   for (j = 0; j  b; j++ ) {
 +   snprintf(buf2 + j * 2, sizeof buf2,
 fmt, file-u.sum[j]);

ditto.

--yoshfuji
-- 
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html


Re: adding a new log-format escape

2005-06-27 Thread Paul Slootman
On Mon 27 Jun 2005, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
 In article [EMAIL PROTECTED] (at Mon, 27 Jun 2005 00:07:19 -0600), Andrew 
 Shewmaker [EMAIL PROTECTED] says:
 
  +   for (j = 0; j  SUM_LENGTH; j++ ) {
  +   snprintf(buf2 + j * 2, sizeof buf2,
  fmt, file-u.sum[j]);
 
file-u.sum[j]  0xff
 
  +   for (j = 0; j  b; j++ ) {
  +   snprintf(buf2 + j * 2, sizeof buf2,
  fmt, file-u.sum[j]);
 
 ditto.

Note also that to preserve the protection offered by snprintf, the
sizeof buf2 needs to be changed to sizeof buf2 - j * 2.


Paul Slootman
-- 
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html


Re: adding a new log-format escape

2005-06-27 Thread Wayne Davison
On Mon, Jun 27, 2005 at 12:07:19AM -0600, Andrew Shewmaker wrote:
 +   for (j = 0; j  SUM_LENGTH; j++ ) {
 +   snprintf(buf2 + j * 2, sizeof buf2, fmt, 
 file-u.sum[j]);

Not every entry will have a non-zero sum pointer (e.g. symlinks, dirs,
etc.), so you need to be sure to check for this and print some other
value (N/A) for entries with a NULL pointer.

..wayne..
-- 
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html


Re: adding a new log-format escape

2005-06-27 Thread Wayne Davison
On Mon, Jun 27, 2005 at 09:22:25AM -0700, Wayne Davison wrote:
 so you need to be sure to check for this and print some other
 value (N/A) for entries with a NULL pointer.

Actually, the proper fix is to check the mode using S_ISREG() and only
ever dereference u.sum for a regular file (since other item types use
that union for other purposes).

..wayne..
-- 
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html


Re: adding a new log-format escape

2005-06-27 Thread Andrew Shewmaker
On 6/27/05, Paul Slootman [EMAIL PROTECTED] wrote:
 On Mon 27 Jun 2005, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
  In article [EMAIL PROTECTED] (at Mon, 27 Jun 2005 00:07:19 -0600), Andrew 
  Shewmaker [EMAIL PROTECTED] says:
 
   +   for (j = 0; j  SUM_LENGTH; j++ ) {
   +   snprintf(buf2 + j * 2, sizeof buf2,
   fmt, file-u.sum[j]);
 
 file-u.sum[j]  0xff
 
   +   for (j = 0; j  b; j++ ) {
   +   snprintf(buf2 + j * 2, sizeof buf2,
   fmt, file-u.sum[j]);
 
  ditto.
 
 Note also that to preserve the protection offered by snprintf, the
 sizeof buf2 needs to be changed to sizeof buf2 - j * 2.

Ahh, thanks Yoshfugi.  I thought I had checked that file-u.sum was an
unsigned character array.  Also, thank you, Paul, for pointing out my
snprintf mistake.

-- 
Andrew Shewmaker
--
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html


Re: adding a new log-format escape

2005-06-27 Thread Andrew Shewmaker
On 6/27/05, Wayne Davison [EMAIL PROTECTED] wrote:
 On Mon, Jun 27, 2005 at 09:22:25AM -0700, Wayne Davison wrote:
  so you need to be sure to check for this and print some other
  value (N/A) for entries with a NULL pointer.
 
 Actually, the proper fix is to check the mode using S_ISREG() and only
 ever dereference u.sum for a regular file (since other item types use
 that union for other purposes).

Okay, I've got it now.  Thanks for your help.  Here's what I ended up
with in case anyone's interested.

--- rsync-2.6.5-orig/log.c  2005-04-14 10:08:10.0 -0600
+++ rsync-2.6.5/log.c   2005-06-27 10:35:51.511736804 -0600
@@ -371,7 +371,7 @@
char buf[MAXPATHLEN+1024], buf2[MAXPATHLEN], fmt[32];
char *p, *s, *n;
size_t len, total;
-   int64 b;
+   int64 b, j;

*fmt = '%';

@@ -483,6 +483,17 @@
snprintf(buf2, sizeof buf2, fmt, (double)b);
n = buf2;
break;
+   case 's':
+   if (S_ISREG(file-mode)) {
+   strlcat(fmt, 02x, sizeof fmt);
+   for (j = 0; j  SUM_LENGTH; j++ )
+   snprintf(buf2 + j * 2, sizeof
buf2 - j * 2, fmt, (uchar) file-u.sum[j]);
+   } else {
+   strlcat(fmt, s, sizeof fmt);
+   snprintf(buf2, sizeof buf2, fmt, N/A);
+   }
+   n = buf2;
+   break;
case 'i':
if (iflags  ITEM_DELETED) {
n = *deleting;

--- rsync-2.6.5-orig/rsyncd.conf.yo 2005-06-01 21:54:45.0 -0600
+++ rsync-2.6.5/rsyncd.conf.yo  2005-06-27 10:52:51.754131476 -0600
@@ -406,6 +406,7 @@
   it() %b for the number of bytes actually transferred
   it() %c when sending files this gives the number of checksum bytes
 received for this file
+  it() %s for the checksum of a file
   it() %i an itemized list of what is being updated
 ))

-- 
Andrew Shewmaker
--
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html


Re: adding a new log-format escape

2005-06-27 Thread Wayne Davison
On Mon, Jun 27, 2005 at 10:56:41AM -0600, Andrew Shewmaker wrote:
 +   strlcat(fmt, 02x, sizeof fmt);

The purpose of the fmt buffer is to allow the user to specify something
like %-20s and get the output formatted into their log line, so you
shouldn't be using the fmt value for internal purposes.  This code would
be better:

if (S_ISREG(file-mode)) {
n = buf2 + MAXPATHLEN - SUM_LENGTH * 2 - 1;
for (j = 0; j  SUM_LENGTH; j++)
sprintf(n + j * 2, %02x, (uchar) file-u.sum[j]);
} else
n = N/A;

This lets the existing code handle the fmt processing.  (The first
half of that if puts the 32 bytes of checksum data into the tail end
of buf2 and then lets the existing code format it into the start of
buf2.)

..wayne..
-- 
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html