{"thread":{"id":"22113","subject":"base85: Two tiny fixes","startedAt":"2010-01-07T14:58:29Z","lastAt":"2010-01-08T23:15:40Z","messageCount":11,"participants":["Andreas Gruenbacher","Nicolas Pitre","Michael J Gruber","Junio C Hamano","A Large Angry SCM"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"130999","messageId":"201001071558.30065.agruen@suse.de","threadId":"22113","inReplyTo":null,"subject":"base85: Two tiny fixes","fromName":"Andreas Gruenbacher","fromEmail":"agruen@suse.de","sentAt":"2010-01-07T14:58:29Z","receivedAt":"2010-01-07T14:58:29Z","isPatch":false,"sender":{"key":"agruen@suse.de","avatar":null},"body":"While looking at the base85 code I found a bug in the debug code and an \nunnecessary call. You may want to have a look at the two fixes here:\n\n http://www.kernel.org/pub/scm/linux/kernel/git/agruen/git.git\n\nThere is another little oddity in the way the de85 table is set up: 0 \nindicates an invalid entry; to avoid this from clashing with a valid entry, \nvalid entries are incremented by one and decremented again while decoding. \nThis leads to slightly worse code than using a negative number to indicate \ninvalid values (and avoiding to increment/decrement).\n\nAndreas\n"},{"id":"131020","messageId":"alpine.LFD.2.00.1001071253400.21025@xanadu.home","threadId":"22113","inReplyTo":"201001071558.30065.agruen@suse.de","subject":"Re: base85: Two tiny fixes","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2010-01-07T17:58:01Z","receivedAt":"2010-01-07T17:58:01Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 7 Jan 2010, Andreas Gruenbacher wrote:\n\n> While looking at the base85 code I found a bug in the debug code and an \n> unnecessary call. You may want to have a look at the two fixes here:\n> \n> http://www.kernel.org/pub/scm/linux/kernel/git/agruen/git.git\n\nACK. Please post them to this list.\n\n> There is another little oddity in the way the de85 table is set up: 0 \n> indicates an invalid entry; to avoid this from clashing with a valid entry, \n> valid entries are incremented by one and decremented again while decoding. \n> This leads to slightly worse code than using a negative number to indicate \n> invalid values (and avoiding to increment/decrement).\n\nYou can make a patch to modify that as well if you wish. And in that \ncase don't forget to make de85 explicitly signed as a char is unsigned \nby default on some platforms.\n\n\nNicolas\n"},{"id":"131083","messageId":"201001081402.02476.agruen@suse.de","threadId":"22113","inReplyTo":"alpine.LFD.2.00.1001071253400.21025@xanadu.home","subject":"Re: base85: Two tiny fixes","fromName":"Andreas Gruenbacher","fromEmail":"agruen@suse.de","sentAt":"2010-01-08T13:02:02Z","receivedAt":"2010-01-08T13:02:02Z","isPatch":false,"sender":{"key":"agruen@suse.de","avatar":null},"body":"On Thursday 07 January 2010 18:58:01 Nicolas Pitre wrote:\n> ACK. Please post them to this list.\n\nOkay, done.\n\n> On Thu, 7 Jan 2010, Andreas Gruenbacher wrote:\n> > There is another little oddity in the way the de85 table is set up: 0 \n> > indicates an invalid entry; to avoid this from clashing with a valid entry, \n> > valid entries are incremented by one and decremented again while decoding. \n> > This leads to slightly worse code than using a negative number to indicate \n> > invalid values (and avoiding to increment/decrement).\n> \n> You can make a patch to modify that as well if you wish.\n\nNah, it's not worth the noise.\n\n> And in that case don't forget to make de85 explicitly signed as a char is\n> unsigned by default on some platforms.\n\nI would have forgotten this; thanks for pointing it out!\n\nThanks,\nAndreas\n"},{"id":"131087","messageId":"1262958000-27181-1-git-send-email-agruen@suse.de","threadId":"22113","inReplyTo":"alpine.LFD.2.00.1001071253400.21025@xanadu.home","subject":"[PATCH 1/3] base85 debug code: Fix length byte calculation","fromName":"Andreas Gruenbacher","fromEmail":"agruen@suse.de","sentAt":"2010-01-08T13:39:58Z","receivedAt":"2010-01-08T13:39:58Z","isPatch":true,"sender":{"key":"agruen@suse.de","avatar":null},"body":"Signed-off-by: Andreas Gruenbacher \n---\n base85.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/base85.c b/base85.c\nindex b417a15..1d165d9 100644\n--- a/base85.c\n+++ b/base85.c\n@@ -118,7 +118,7 @@ int main(int ac, char **av)\n \t\tint len = strlen(av[2]);\n \t\tencode_85(buf, av[2], len);\n \t\tif (len <= 26) len = len + 'A' - 1;\n-\t\telse len = len + 'a' - 26 + 1;\n+\t\telse len = len + 'a' - 26 - 1;\n \t\tprintf(\"encoded: %c%s\\n\", len, buf);\n \t\treturn 0;\n \t}\n-- \n1.6.6.75.g37bae\n"},{"id":"131088","messageId":"1262958000-27181-2-git-send-email-agruen@suse.de","threadId":"22113","inReplyTo":"alpine.LFD.2.00.1001071253400.21025@xanadu.home","subject":"[PATCH 2/3] base85: No need to initialize the decode table in encode_85","fromName":"Andreas Gruenbacher","fromEmail":"agruen@suse.de","sentAt":"2010-01-08T13:39:59Z","receivedAt":"2010-01-08T13:39:59Z","isPatch":true,"sender":{"key":"agruen@suse.de","avatar":null},"body":"Signed-off-by: Andreas Gruenbacher \n---\n base85.c | 2 --\n 1 files changed, 0 insertions(+), 2 deletions(-)\n\ndiff --git a/base85.c b/base85.c\nindex 1d165d9..7204ce2 100644\n--- a/base85.c\n+++ b/base85.c\n@@ -84,8 +84,6 @@ int decode_85(char *dst, const char *buffer, int len)\n \n void encode_85(char *buf, const unsigned char *data, int bytes)\n {\n-\tprep_base85();\n-\n \tsay(\"encode 85\");\n \twhile (bytes) {\n \t\tunsigned acc = 0;\n-- \n1.6.6.75.g37bae\n"},{"id":"131086","messageId":"1262958000-27181-3-git-send-email-agruen@suse.de","threadId":"22113","inReplyTo":"alpine.LFD.2.00.1001071253400.21025@xanadu.home","subject":"[PATCH 3/3] base85: Make the code more obvious instead of explaining the non-obvious","fromName":"Andreas Gruenbacher","fromEmail":"agruen@suse.de","sentAt":"2010-01-08T13:40:00Z","receivedAt":"2010-01-08T13:40:00Z","isPatch":true,"sender":{"key":"agruen@suse.de","avatar":null},"body":"Here is another cleanup ...\n\nSigned-off-by: Andreas Gruenbacher \n---\n base85.c | 10 ++--------\n 1 files changed, 2 insertions(+), 8 deletions(-)\n\ndiff --git a/base85.c b/base85.c\nindex 7204ce2..e459fee 100644\n--- a/base85.c\n+++ b/base85.c\n@@ -57,14 +57,8 @@ int decode_85(char *dst, const char *buffer, int len)\n \t\tde = de85[ch];\n \t\tif (--de < 0)\n \t\t\treturn error(\"invalid base85 alphabet %c\", ch);\n-\t\t/*\n-\t\t * Detect overflow. The largest\n-\t\t * 5-letter possible is \"|NsC0\" to\n-\t\t * encode 0xffffffff, and \"|NsC\" gives\n-\t\t * 0x03030303 at this point (i.e.\n-\t\t * 0xffffffff = 0x03030303 * 85).\n-\t\t */\n-\t\tif (0x03030303 < acc ||\n+\t\t/* Detect overflow. */\n+\t\tif (0xffffffff / 85 < acc ||\n \t\t 0xffffffff - de < (acc *= 85))\n \t\t\treturn error(\"invalid base85 sequence %.5s\", buffer-5);\n \t\tacc += de;\n-- \n1.6.6.75.g37bae\n"},{"id":"131098","messageId":"4B475361.60506@drmicha.warpmail.net","threadId":"22113","inReplyTo":"1262958000-27181-2-git-send-email-agruen@suse.de","subject":"Re: [PATCH 2/3] base85: No need to initialize the decode table in encode_85","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-01-08T15:46:41Z","receivedAt":"2010-01-08T15:46:41Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Andreas Gruenbacher venit, vidit, dixit 08.01.2010 14:39:\n> Signed-off-by: Andreas Gruenbacher \n> ---\n\nFor the less informed it may be worthwhile to have an explanation in the\ncommit message why encode_85() does not need to initialize the table. (I\nstrongly suspect it's a matter of de vs. en, i.e. \"because it only\nencodes but does not decode.\"...)\n\n> base85.c | 2 --\n> 1 files changed, 0 insertions(+), 2 deletions(-)\n> \n> diff --git a/base85.c b/base85.c\n> index 1d165d9..7204ce2 100644\n> --- a/base85.c\n> +++ b/base85.c\n> @@ -84,8 +84,6 @@ int decode_85(char *dst, const char *buffer, int len)\n> \n> void encode_85(char *buf, const unsigned char *data, int bytes)\n> {\n> -\tprep_base85();\n> -\n> \tsay(\"encode 85\");\n> \twhile (bytes) {\n> \t\tunsigned acc = 0;\n"},{"id":"131099","messageId":"7v3a2giobm.fsf@alter.siamese.dyndns.org","threadId":"22113","inReplyTo":"4B475361.60506@drmicha.warpmail.net","subject":"Re: [PATCH 2/3] base85: No need to initialize the decode table in encode_85","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-08T15:55:09Z","receivedAt":"2010-01-08T15:55:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Andreas Gruenbacher venit, vidit, dixit 08.01.2010 14:39:\n>> Signed-off-by: Andreas Gruenbacher \n>> ---\n>\n> For the less informed it may be worthwhile to have an explanation in the\n> commit message why encode_85() does not need to initialize the table. (I\n> strongly suspect it's a matter of de vs. en, i.e. \"because it only\n> encodes but does not decode.\"...)\n\nThe title can be reworded to\n\n base85: encode85() does not use the decode table\n"},{"id":"131104","messageId":"1262967470-31782-1-git-send-email-agruen@suse.de","threadId":"22113","inReplyTo":"7v3a2giobm.fsf@alter.siamese.dyndns.org","subject":"[PATCH] base85: encode85() does not use the decode table","fromName":"Andreas Gruenbacher","fromEmail":"agruen@suse.de","sentAt":"2010-01-08T16:17:50Z","receivedAt":"2010-01-08T16:17:50Z","isPatch":true,"sender":{"key":"agruen@suse.de","avatar":null},"body":"Signed-off-by: Andreas Gruenbacher \n---\n base85.c | 2 --\n 1 files changed, 0 insertions(+), 2 deletions(-)\n\ndiff --git a/base85.c b/base85.c\nindex 1d165d9..7204ce2 100644\n--- a/base85.c\n+++ b/base85.c\n@@ -84,8 +84,6 @@ int decode_85(char *dst, const char *buffer, int len)\n \n void encode_85(char *buf, const unsigned char *data, int bytes)\n {\n-\tprep_base85();\n-\n \tsay(\"encode 85\");\n \twhile (bytes) {\n \t\tunsigned acc = 0;\n-- \n1.6.6.75.g37bae\n"},{"id":"131106","messageId":"1262967738-31996-1-git-send-email-agruen@suse.de","threadId":"22113","inReplyTo":"7v3a2giobm.fsf@alter.siamese.dyndns.org","subject":"[PATCH] base85: encode_85() does not use the decode table","fromName":"Andreas Gruenbacher","fromEmail":"agruen@suse.de","sentAt":"2010-01-08T16:22:18Z","receivedAt":"2010-01-08T16:22:18Z","isPatch":true,"sender":{"key":"agruen@suse.de","avatar":null},"body":"Signed-off-by: Andreas Gruenbacher \n---\n base85.c | 2 --\n 1 files changed, 0 insertions(+), 2 deletions(-)\n\ndiff --git a/base85.c b/base85.c\nindex 1d165d9..7204ce2 100644\n--- a/base85.c\n+++ b/base85.c\n@@ -84,8 +84,6 @@ int decode_85(char *dst, const char *buffer, int len)\n \n void encode_85(char *buf, const unsigned char *data, int bytes)\n {\n-\tprep_base85();\n-\n \tsay(\"encode 85\");\n \twhile (bytes) {\n \t\tunsigned acc = 0;\n-- \n1.6.6.75.g37bae\n"},{"id":"131143","messageId":"4B47BC9C.1020807@gmail.com","threadId":"22113","inReplyTo":"1262958000-27181-3-git-send-email-agruen@suse.de","subject":"Re: [PATCH 3/3] base85: Make the code more obvious instead of explaining the non-obvious","fromName":"A Large Angry SCM","fromEmail":"gitzilla@gmail.com","sentAt":"2010-01-08T23:15:40Z","receivedAt":"2010-01-08T23:15:40Z","isPatch":true,"sender":{"key":"gitzilla@gmail.com","avatar":"https://gravatar.com/avatar/354625c442439908ff3dd99757dee330e29e9df7847472384faf7a00add247fb?d=mp&s=160"},"body":"Andreas Gruenbacher wrote:\n> Here is another cleanup ...\n> \n\nI LOVE the subject line of this commit!\n"}]}