Quoth Mark Walters on Jul 25 at 7:57 pm:
>
> Hi
>
> I have read this series (apart from the test changes) and it is both
> very nice to review and looks good. It builds and all tests pass at all
> the interesting partial stages and when fully applied. I have a few very
> minor comments.
>
> On Wed, 25 Jul 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> > This method allows callers to output strings with specific lengths.
> > It's useful both for strings with embedded NULs (which JSON can
> > represent, though parser support is apparently spotty), and
> > non-terminated strings.
> > ---
> > sprinter-json.c | 11 +++++++++--
> > sprinter-text.c | 11 +++++++++--
> > sprinter.h | 1 +
> > 3 files changed, 19 insertions(+), 4 deletions(-)
> >
> > diff --git a/sprinter-json.c b/sprinter-json.c
> > index 4649655..2587ca6 100644
> > --- a/sprinter-json.c
> > +++ b/sprinter-json.c
> > @@ -89,7 +89,7 @@ json_end (struct sprinter *sp)
> > }
> >
> > static void
> > -json_string (struct sprinter *sp, const char *val)
> > +json_string_len (struct sprinter *sp, const char *val, size_t len)
> > {
>
> I think this function could do with a comment along the lines of the
> commit message. It might be nice to document somewhere where/when we
Will do. I put a comment on this function explaining how it handles
NULs, as well as a comment on the generic vtable pointer explaining
string_len.
> might actually have nulls in an encoded string (are they allowed in
> bodies, headers etc).
Interestingly, RFC822, while being limited to 7-bit ASCII, explicitly
did allow NULs [RFC822, CHAR non-terminal]. RFC2822 explicitly
obsoletes them [RFC2822 4 or appendix B], which means consumers should
support them, but generators must not generate them. As usual, MIME
makes things more complicated by supporting a "binary" content
transfer encoding that does permits NULs. However, good luck getting
that through SMTP business; SMTP theoretically allows any ASCII
control character, but the standard specifically warns that control
characters other than SP, HT, CR, and LF should be avoided [RFC2821
4.1.1.4]. Even if you've negotiated the 8BITMIME extension [RFC1652],
you're limited by what MIME's "8bit" content transfer encoding
permits, which differs from the "binary" content transfer encoding by
disallowing (guess what) NULs and long lines.
In other words: yes.
> Actually, do we know that the json emacs parser handles nulls correctly?
It does. Emacs strings are clean, so json.el doesn't have to do
special anything to support them.
> Best wishes
>
> Mark
> > static const char *const escapes[] = {
> > ['\"'] = "\\\"", ['\\'] = "\\\\", ['\b'] = "\\b",
> > @@ -98,7 +98,7 @@ json_string (struct sprinter *sp, const char *val)
> > struct sprinter_json *spj = json_begin_value (sp);
> >
> > fputc ('"', spj->stream);
> > - for (; *val; ++val) {
> > + for (; len; ++val, --len) {
> > unsigned char ch = *val;
> > if (ch < ARRAY_SIZE (escapes) && escapes[ch])
> > fputs (escapes[ch], spj->stream);
> > @@ -111,6 +111,12 @@ json_string (struct sprinter *sp, const char *val)
> > }
> >
> > static void
> > +json_string (struct sprinter *sp, const char *val)
> > +{
> > + json_string_len (sp, val, strlen (val));
> > +}
> > +
> > +static void
> > json_integer (struct sprinter *sp, int val)
> > {
> > struct sprinter_json *spj = json_begin_value (sp);
> > @@ -166,6 +172,7 @@ sprinter_json_create (const void *ctx, FILE *stream)
> > .begin_list = json_begin_list,
> > .end = json_end,
> > .string = json_string,
> > + .string_len = json_string_len,
> > .integer = json_integer,
> > .boolean = json_boolean,
> > .null = json_null,
> > diff --git a/sprinter-text.c b/sprinter-text.c
> > index b208840..dfa54b5 100644
> > --- a/sprinter-text.c
> > +++ b/sprinter-text.c
> > @@ -25,14 +25,20 @@ struct sprinter_text {
> > };
> >
> > static void
> > -text_string (struct sprinter *sp, const char *val)
> > +text_string_len (struct sprinter *sp, const char *val, size_t len)
> > {
> > struct sprinter_text *sptxt = (struct sprinter_text *) sp;
> >
> > if (sptxt->current_prefix != NULL)
> > fprintf (sptxt->stream, "%s:", sptxt->current_prefix);
> >
> > - fputs(val, sptxt->stream);
> > + fwrite (val, len, 1, sptxt->stream);
> > +}
> > +
> > +static void
> > +text_string (struct sprinter *sp, const char *val)
> > +{
> > + text_string_len (sp, val, strlen (val));
> > }
> >
> > static void
> > @@ -105,6 +111,7 @@ sprinter_text_create (const void *ctx, FILE *stream)
> > .begin_list = text_begin_list,
> > .end = text_end,
> > .string = text_string,
> > + .string_len = text_string_len,
> > .integer = text_integer,
> > .boolean = text_boolean,
> > .null = text_null,
> > diff --git a/sprinter.h b/sprinter.h
> > index 6680d41..826a852 100644
> > --- a/sprinter.h
> > +++ b/sprinter.h
> > @@ -28,6 +28,7 @@ typedef struct sprinter {
> > * For string, the char * must be UTF-8 encoded.
> > */
> > void (*string) (struct sprinter *, const char *);
> > + void (*string_len) (struct sprinter *, const char *, size_t);
> > void (*integer) (struct sprinter *, int);
> > void (*boolean) (struct sprinter *, notmuch_bool_t);
> > void (*null) (struct sprinter *);
> >
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch
>
--
Austin Clements MIT/'06/PhD/CSAIL
amdragon@mit.edu http://web.mit.edu/amdragon
Somewhere in the dream we call reality you will find me,
searching for the reality we call dreams.