From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
To: David Bremner <david@tethera.net>, notmuch@notmuchmail.org
Cc: David Bremner <bremner@debian.org>, pere@hungry.com
Subject: Re: [Alpha Patch 2/2] test: update dump-restore tests for 8 bit characters.
Date: Thu, 15 Dec 2011 00:36:38 +0400 [thread overview]
Message-ID: <87pqfqubzd.fsf@gmail.com> (raw)
In-Reply-To: <1323893641-4677-2-git-send-email-david@tethera.net>
On Wed, 14 Dec 2011 16:14:01 -0400, David Bremner <david@tethera.net> wrote:
> From: David Bremner <bremner@debian.org>
>
> hex-escape: fix for handling of 8 bit chars
>
> The low level problem was passing negative numbers to sprintf(s,"%x");
> we fix this and clarify the api for hex_(decode|encode) by making
> encode go from (unsigned char *) (i.e. 8bit) to (char *) and decode
> vise-versa.
I did not do a proper review. But I think the encoder and decoder
should accept and return the same type, either char* or unsigned char*.
The decision should be based on what type strings (that would be fed to
the encoder and decoder) have in notmuch code. I guess it is char*, so
the encoder and decoder should take and return char*. Internally we
would cast char* to unsigned char*.
Also, I do not like the _octet typedef in hex-escape.c. Having
different function parameters in header and .c is confusing. IMO we
should either move the typedef to some header, or just use unsigned
char.
Regards,
Dmitry
> ---
> test/dump-restore | 2 --
> test/hex-escaping | 1 -
> util/hex-escape.c | 26 +++++++++++++++-----------
> util/hex-escape.h | 6 ++++--
> 4 files changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/test/dump-restore b/test/dump-restore
> index eee1773..c5b2e86 100755
> --- a/test/dump-restore
> +++ b/test/dump-restore
> @@ -114,7 +114,6 @@ notmuch dump --format=notmuch > BACKUP
> notmuch tag +"$tag1" +"$tag2" +"$tag3" -inbox -unread "*"
>
> test_begin_subtest 'format=notmuch, round trip with strange tags'
> - test_subtest_known_broken
> notmuch dump --format=notmuch > EXPECTED.$test_count
> notmuch dump --format=notmuch | notmuch restore --format=notmuch
> notmuch dump --format=notmuch > OUTPUT.$test_count
> @@ -122,7 +121,6 @@ test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
>
>
> test_begin_subtest 'format=notmuch, checking encoded output'
> - test_subtest_known_broken
> cp /dev/null EXPECTED.$test_count
> notmuch dump --format=notmuch -- from:cworth |\
> awk "{ print \$1 \" $enc1 $enc2 $enc3\" }" > EXPECTED.$test_count
> diff --git a/test/hex-escaping b/test/hex-escaping
> index 2053fb0..daa6446 100755
> --- a/test/hex-escaping
> +++ b/test/hex-escaping
> @@ -19,7 +19,6 @@ $TEST_DIRECTORY/hex-xcode e < EXPECTED.$test_count |\
> test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
>
> test_begin_subtest "round trip 8bit chars"
> -test_subtest_known_broken
> echo '%c3%91%c3%a5%c3%b0%c3%a3%c3%a5%c3%a9-%c3%8f%c3%8a' > EXPECTED.$test_count
> $TEST_DIRECTORY/hex-xcode d < EXPECTED.$test_count |\
> $TEST_DIRECTORY/hex-xcode e > OUTPUT.$test_count
> diff --git a/util/hex-escape.c b/util/hex-escape.c
> index dcf87cf..565ae99 100644
> --- a/util/hex-escape.c
> +++ b/util/hex-escape.c
> @@ -28,23 +28,24 @@ static const size_t default_buf_size=1024;
> static const char* output_charset=
> "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+-_@=.:,";
>
> -static const char escape_char='%';
> +static const int escape_char = '%';
>
> static int
> is_output (char c) {
> return (strchr (output_charset, c) != NULL);
> }
>
> +typedef unsigned char _octet;
>
> static int
> -maybe_realloc(void *ctx, size_t needed, char **out, size_t *out_size)
> +maybe_realloc(void *ctx, size_t needed, _octet **out, size_t *out_size)
> {
> if (*out_size < needed) {
>
> if (*out == NULL)
> *out = talloc_size(ctx,needed);
> else
> - *out = talloc_realloc(ctx,*out,char,needed);
> + *out = talloc_realloc(ctx, *out, _octet, needed);
>
> if (*out == NULL)
> return 0;
> @@ -56,24 +57,27 @@ maybe_realloc(void *ctx, size_t needed, char **out, size_t *out_size)
>
>
> hex_status_t
> -hex_encode (void *ctx, const char *in, char **out, size_t *out_size)
> +hex_encode (void *ctx, const _octet *in, char **out, size_t *out_size)
> {
>
> - const char *p;
> + const _octet *p;
> char *q;
>
> - int escape_count=0;
> + size_t escape_count = 0;
> + size_t len = 0;
> size_t needed;
>
> - for (p = in; *p; p++)
> + for (p = in; *p; p++) {
> escape_count += (! is_output (*p));
> + len++;
> + }
>
> - needed = strlen (in) + 2*escape_count + 1;
> + needed = len + 2*escape_count + 1;
>
> if (*out == NULL)
> *out_size=0;
>
> - if (!maybe_realloc (ctx, needed, out, out_size))
> + if (!maybe_realloc (ctx, needed, (_octet**)out, out_size))
> return HEX_OUT_OF_MEMORY;
>
> q = *out;
> @@ -94,12 +98,12 @@ hex_encode (void *ctx, const char *in, char **out, size_t *out_size)
>
>
> hex_status_t
> -hex_decode (void *ctx, const char *in, char **out, size_t *out_size) {
> +hex_decode (void *ctx, const char *in, _octet **out, size_t *out_size) {
>
> char buf[3];
>
> const char *p;
> - char *q;
> + _octet *q;
>
> size_t escape_count = 0;
> size_t needed = 0;
> diff --git a/util/hex-escape.h b/util/hex-escape.h
> index 98ecbe0..e04aff5 100644
> --- a/util/hex-escape.h
> +++ b/util/hex-escape.h
> @@ -8,8 +8,10 @@ typedef enum hex_status {
> } hex_status_t;
>
> hex_status_t
> -hex_encode (void *talloc_ctx, const char *in, char **out, size_t *out_size);
> +hex_encode (void *talloc_ctx, const unsigned char *in, char **out,
> + size_t *out_size);
>
> hex_status_t
> -hex_decode (void *talloc_ctx, const char *in, char **out, size_t *out_size);
> +hex_decode (void *talloc_ctx, const char *in, unsigned char **out,
> + size_t *out_size);
> #endif
> --
> 1.7.7.3
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
next prev parent reply other threads:[~2011-12-14 20:37 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-13 20:27 New Dump/Restore Format David Bremner
2011-12-13 20:27 ` [Alpha PATCH 1/6] util/hex-escape.[ch]: encoding/decoding strings into restricted character set David Bremner
2011-12-13 20:27 ` [Alpha PATCH 2/6] test: add test for hex_(encode|decode) David Bremner
2011-12-13 20:27 ` [Alpha PATCH 3/6] notmuch-dump: add --format=(notmuch|sup) David Bremner
2011-12-13 20:27 ` [Alpha PATCH 4/6] test: add test for dump --format=notmuch David Bremner
2011-12-13 20:27 ` [Alpha PATCH 5/6] notmuch-restore: add --format=notmuch support David Bremner
2011-12-13 20:27 ` [Alpha PATCH 6/6] test: second set of dump/restore --format=notmuch tests David Bremner
2011-12-14 20:14 ` [Alpha Patch 1/2] test: add (currently broken) test 8 bit characters hex-escape and dump-restore David Bremner
2011-12-14 20:14 ` [Alpha Patch 2/2] test: update dump-restore tests for 8 bit characters David Bremner
2011-12-14 20:36 ` Dmitry Kurochkin [this message]
2011-12-14 23:20 ` David Bremner
2011-12-14 20:24 ` [Alpha PATCH 6/6] test: second set of dump/restore --format=notmuch tests Dmitry Kurochkin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://notmuchmail.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87pqfqubzd.fsf@gmail.com \
--to=dmitry.kurochkin@gmail.com \
--cc=bremner@debian.org \
--cc=david@tethera.net \
--cc=notmuch@notmuchmail.org \
--cc=pere@hungry.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
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 read-only IMAP folder(s) and NNTP newsgroup(s).