Re: [9front] [PATCH] upas: allow send to parse e-mail descriptions like marshal

[Jacob Moody <moody@posixcafe.org>]

On 6/18/24 13:05, Romano wrote:
> Ping. Here's a cleaned up patch:
> http://okturing.com/src/20104/body
> 
> Similarly with the ramfs patch, any comments/concerns?

The general idea seems fine I think.
I don't use upas, but I did find some
issues with the implementation purposed:

+char *
Typical style is to make the return type 'char*' here

+userfrom(char *cp)
+{
+	char *s;
+	int n;
+
+	if((n = strlen(cp)) > 4 && cp[n-1] == '>'){
+		if((s = strrchr(cp, '<')) != nil && s != cp && isspace(s[-1])) {
+			s++;
+			cp[n-1] = '\0';
+			strcpy(cp, s);

I don't love this loop, we're essentially walking the string three times here.
Once for the first strlen, again for strrchr, and a third time with the strcpy.
Because we return a pointer here we could get rid of this strcpy entirely and just
make this inner block two lines like so:
+			cp[n-1] = '\0';
+			return s + 1;

However at this point we'd still be walking the string twice. I wanted to see if
we could just walk the string once without having the code suck and this is what
I came up with:

char*
userfrom(char *cp)
{
	char *p, *e;

	if((p = strchr(cp, '<')) == nil)
		return cp;
	if(p == cp || !isspace(p[-1]))
		return cp;
	p++;
	if((e = strchr(p, '>')) == nil)
		return cp;
	*e = '\0';
	return p;
}

Not that the length is going to be super long here, but being a bit more efficient doesn't
make the code any more complicated (imo).

Also I believe the commit message is incorrect given it's understanding of what
printform is doing. I pulled the code out to play with it, and from what I see
when it is given a "A name <a@example.com>" it will print out the full thing,
not just the email. The point of this code seems to ensure that the "name" portion
of this description is passed through the rfc2047fmt function (installed as %U).

/sys/src/cmd/upas/marshal/marshal.c:820
Specifically prints both portions there, with the only difference from the default
case being that it passes the name through %U.

So I think the description of what is going on in the commit message is not quite
correct either.

Thanks,
moody