Re: first stab at simple mailer

[Alexander Shiryaev <coumarin@xxxxxxxxx>]

Simon 'corecode' Schubert wrote:
>
> comments welcome!  (and contributions as well)
>

Don't use GOTO.

For example, this code


	it->sender = sender;
	if ((host = strrchr(it->addr, '@')) != NULL) {
		/* Remote address */
		if (strcmp(host + 1, hostname()) == 0 ||
		    strcmp(host + 1, "localhost") == 0) {
			/* It is really a local thingy */
			*host = 0;
			goto local;
		}
		/* XXX validate address/host? */
		it->remote = 1;
	} else {
local:
		/* Local destination, check */
		pw = getpwnam(it->addr);
		if (expand && pw == NULL)
			goto out;
		it->remote = 0;
		endpwent();
	}
	LIST_FOREACH(tit, &queue->queue, next) {
		/* weed out duplicate dests */
		if (strcmp(tit->addr, it->addr) == 0) {
			free(it->addr);
			free(it);
			return (0);
		}
	}
	LIST_INSERT_HEAD(&queue->queue, it, next);
	return (0);

out:
	free(it->addr);
	free(it);
	return (-1);


should be replaced with


	it->sender = sender;
	host = strrchr(it->addr, '@');
	if ((host != NULL) && (strcmp(host + 1, "localhost") != 0) && 
(strcmp(host + 1, hostname()) != 0)) {
		/* remote */
		/* XXX validate address/host? */
		it->remote = 1;
	} else if (!expand || (Getpwnam(it->addr) != NULL)) {
		/* local */
		it->remote = 0;
		LIST_FOREACH(tit, &queue->queue, next) {
			/* weed out duplicate dests */
			if (strcmp(tit->addr, it->addr) == 0) {
				free(it->addr);
				free(it);
				return (0);
			}
		}
		LIST_INSERT_HEAD(&queue->queue, it, next);
		return (0);
	}
	free(it->addr);
	free(it);
	return (-1);


Best regards, Alexander.