bug-mailutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: The state I'm in - rfc 822 parsing.


From: Alain Magloire
Subject: Re: The state I'm in - rfc 822 parsing.
Date: Tue, 13 Mar 2001 23:21:38 -0500 (EST)

Bonjour

> Anyhow, I'm parsing everything that's allowed, I think,
> plus the comment at the end of an addr-spec (when it's
> at the end of a mailbox).
> 
> I'm particularly proud of:
> 
> Sam <@[matrix (smtp)], @[nexus: \[node 12\]]:address@hidden>
> 

Wow! very cool, the old parser would have serious chock
on most test in Addrs.

> The old address-create() is still there, and mine is called
> address_create0(), other than that the apis are the same.
> 
> I've also added apis to get the local-part, the domain,
> and the route.
> 
> If I find the awful comment, I put it where I would have
> put a phrase.

Seems you actually remove comments totaly.
Some clients may want them right.  Especially in the "Received:"

> I accept route-addrs without a phrase in front.
> 
> Groups just look like more addresses, I discard their names.

Ok, suggestion; how about to build on top(not now, but much ... much later)
something that would permit a user to build and save aliases:
alias_create (&alias, "canadian_hackers");
alias_add (alias, "address@hidden");
alias_add (alias, "address@hidden");
...

>  (in the attached files, search for "b- ", looks like the
>  author of qmail has a sense of humour)

8-), If I remember djb, he was not easy to deal with.

> <strings.h> is needed under Nto for one of the non-standard
> string functions in mbx_imap.c, so I conditionally included
> it.

Ok.

> Next:
> 
> - I'm ok with this being checked in, I think it lives nicely
> to the side of whats there, so it won't screw anybody, but
> people can play with it. If you want. 8-)

Allright.

> - <>, valid in SMTP "rcpt from:", should I parse and ignore it?

I vote strip. Since I want to reuse the email part to build the "From "
string separator for Unix Mbox.

> - I don't accept > 127 chars as ascii anywhere, should I allow
> them in the personal name?

Technically it should not be permit since rfc822 explicit specify
that header should be ascii7, maybe substitute them with "?"

> Can somebody send me example fields that have them?

Nothing at hand.  I use to receive email from Norway and the spammer
would put is name with funny "ae" etc that would muck the terminal.
The old "flash" attack, the program would generate headers with
binary(none ascii) that would give elm a hardtime and specially harmfull
when you were log via a modem.

> - I don't check for memory failure (duck). My excuse: no

Yes, you could use the "poison" mallocs (xmalloc(), xrealloc, xcalloc)
but I do know if that is the proper way for a library to behave. 

> feedback on whether the reallocations I'm doing make people
> cringe. I.e, whether I need to rework the building up of strings.
> I can think of some other ways to do it, but I don't want to
> if I don't have to.

The realloc-by-1 is hard.  Why not construct a "string class" and realloc
by a fixe size, say 16/32, since it's only short strings.

struct _string {
  char *buffer;
  size_t buflen;
};
and pass around the struct _string *;

Or using GNU obstack object.

> - proper prototypes and naming conventions, maybe even a more
> abstract api for the parser, C++ comments -> C comments, etc.

Some serious nitpicking, 8-) :

- (*)Proper prototypes is a good thing.

- (*)C comments, please, some old compiler will chocke.

- (*)Also not using c++ reserved words, I saw a couple of "char *new;"

- (*)A more consitent use of "const char *" versus "char *"
  for example StrAppend*() should be :
  int StrAppend (char **to, const char *from);

- (*)"new = (char*) realloc()", cast not needed

- I saw a mailbox_t can we rename this for consistency, since
  the name is already taken.

- The return values looks a little inconsistent 0== vs != 0

- (**)GNU coding style: this not a high priority, I probably will not do nothing
  now, since lots of othe code have different style, for example, mime.c.
  It's up to you, some people can not stand it.

- (***)Some memory leaks, when the parsing failed, since it's a library
  we should release, (see append patch, ok to commit?).  

(*)I can submit a patch to correct those if you want.
(**) I can submit a patch but ... you may not like it 8-).
(***) patch in attachement, not commited.

Attachment: parse822.diff
Description: C program text


reply via email to

[Prev in Thread] Current Thread [Next in Thread]