Add git-imap-send.

This probably needs a bit more work, but I'll solicit comments and flames anyway...

git-imap-send drops a patch series generated by git-format-patch into an IMAP folder. This allows patch submitters to send patches through their own mail program.

git-imap-send uses the following values from the GIT repository configuration:

The target IMAP folder:

[imap]
Folder = "INBOX.Drafts"

A command to open an ssh tunnel to the imap mail server.

[imap]
Tunnel = "ssh -q [email protected] /usr/bin/imapd ./Maildir
2> /dev/null"

OR

[imap]
Host = imap.server.com
User = bob
Password = pwd
Port = 143

---

 .gitignore  |    1
 Makefile    |    4
 imap-send.c | 1935 
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1939 insertions(+), 1 deletions(-)
 create mode 100644 imap-send.c

b8fbeb40b92b9b6048c9f7ad66a10d8222c58b76

index abbc509..c567cc0 100644
@@ -42,6 +42,7 @@ git-grep
 git-hash-object
 git-http-fetch
 git-http-push
+git-imap-send
 git-index-pack
 git-init-db
 git-local-fetch

index b6d8804..bc49f06 100644
@@ -165,7 +165,7 @@ PROGRAMS = \
 	git-upload-pack$X git-verify-pack$X git-write-tree$X \
 	git-update-ref$X git-symbolic-ref$X git-check-ref-format$X \
 	git-name-rev$X git-pack-redundant$X git-repo-config$X git-var$X \
-	git-describe$X git-merge-tree$X git-blame$X
+	git-describe$X git-merge-tree$X git-blame$X git-imap-send$X
 
 # what 'all' will build and 'install' will install, in gitexecdir
 ALL_PROGRAMS = $(PROGRAMS) $(SIMPLE_PROGRAMS) $(SCRIPTS)

@@ -522,6 +522,8 @@ git-ssh-upload$X: rsh.o
 git-ssh-pull$X: rsh.o fetch.o
 git-ssh-push$X: rsh.o
 
+git-imap-send$X: imap-send.o $(LIB_FILE)
+
 git-http-fetch$X: fetch.o http.o http-fetch.o $(LIB_FILE)
 	$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
 		$(LIBS) $(CURL_LIBCURL)

OK, then please work a bit more ;-)

I kind of like this approach and even wish my e-mail workflow involved an imap server with draft folders. Currently I do everything in Gnus and my drafts are on local disk.

The .git/config file is a good choice for storing this information, because you might even use different draft folders for different projects, i.e. the configuration is per-repository.

These I am not so sure. It _might_ make sense to have something like this under $HOME/. Isn't there an established convention for storing something like this for existing MUAs?

Hmmm.

We have something like this in apply.c, exec_cmd.c and git.c; probably we would want a macro in "cache.h".

	git.c:326:#define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))

It seems better to keep all the configuration in the same place. It's possible (although unlikely) that somebody uses two different IMAP servers to send mail... maybe one for work and one for play?

I think it would also be good to be able to specify these as command line options, as some people might be uncomfortable with having their password in a file (though it would be better to use an ssh tunnel).

I'll remove this, and all the SSL specific code, as it also complicates the Makefile and adds an extra dependency.

I've fixed all the other issues you pointed out, and will resend the patch.

Mike

Hi,

That does not matter, since they are strings, and the memcmp should not look further if they are shorter (because the comparison to '\0' failed already).

However, if !memcmp("imaps:", val, 6), it means that val starts with the string "imaps:", right? Then !memcmp("imap:", val, 5) must always fail, no?

Ciao,
Dscho

That's what strcmp() does. memcmp() walks the lenghth even if it encounters nul bytes. Perhaps you confuse it with strncmp()?

Yes. Recent gcc's will recognize it as dead code and remove it. It's still ugly though.

Andreas Ericsson                   [email protected]
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Hi,

Sorry, I was unclear. Of course, memcmp() does not stop on NUL. But it stops when that NUL is different from what the other pointer has. Which is the case here.

Ciao,
Dscho

Does it really? My copy of the spec doesn't say that. It says only this:

: 7.21.4.1 The memcmp function
:
: Synopsis
:
: #include <string.h>
: int memcmp(const void *s1, const void *s2, size_t n);
:
: Description
:
: The memcmp() function compares the first n characters of the object : pointed to by s1 to the first n characters of the object pointed to by : s2.262)
:
: Returns
:
: The memcmp function returns an integer greater than, equal to, or less : than zero, accordingly as the object pointed to by s1 is greater than, : equal to, or less than the object pointed to by s2.
:
: 262) The contents of ``holes'' used as padding for purposes of : alignment within structure objects are indeterminate. Strings shorter : than their allocated space and unions may also cause problems in : comparison.

There are reasons for which it'd be desirable that memcmp really compare all the bytes, even if it can in theory stop early: in particular, there are cases where early exit can leak timing information which makes it possible to attack cryptographic protocols.

I'd have to recommend strncmp for this job.

-- [mdw]

Hi,

I would be astonished if memcmp has to be timing proof for *all* applications, just to keep crypto people happy. I don't *want* a slow-down in my super-duper 3d shooter.

Fully agree.

Ciao,
Dscho

Oh, obviously that would be lunatic! ;-) Just making the point that it's not /completely/ obvious that early stopping is what everyone wants...

-- [mdw]

No.

It's true that any sane memcmp() will stop when it notices a difference, and it's also true that the return value semantics of memcmp() means that it has to walk beginning-to-end.

HOWEVER. The key phrase is "_when_ it notices a difference".

It's quite common for optimized memcmp()'s to do things like loading several words from both the source and the destinations, and testing them together, and only start doing the byte-by-byte comparison when the "big" comparison has failed.

So when you do a

	if (!memcmp(string, mystring, mystringlength))
		...

it's entirely possible that it will load bytes from "string" _past_ the end of the string because of an unrolled inner loop that does things multiple bytes at a time. They won't be used in the eventual result, but just the fact that they are loaded from memory can mean that your program takes a SIGSEGV, for example, becaue it turns out "string" was just a single NUL byte at the end of a page, and there's nothing after it.

IOW, it's a bad optimization.

Use "strncmp()" instead. Yes, it can be slower, exactly because it has to check more, but it checks more exactly because memcmp() can cause undefined behaviour by running off the end of a string.

Linus

Funny. I've seen this exact bug in memcmp and strcmp in earlier SunOS (pre Solaris) libc when I was working on something like Valgrind in my previous life.

Well, Valgrind actually can complain for no good reason.

System libraries often take advantage of knowing how the CPU and the system memory layout works. For example, just from knowing that pages are always aligned to a certain (largish) boundary, you can know that it's perfectly safe to do certain optimizations and prefetch past the end of an object, as long as it's in the same page (and the easiest way to verify that is to just do it when something is aligned).

So Valgrind will sometimes complain about perfectly bug-free code, just because the bug-free code accesses outside the "strictly allowable" region because it knows it can.

Valgrind can be taught about system libraries like that, but especially if it's an early port to a new architecture or OS, valgrind will often complain unnecessarily.

Linus

To clarify, it was _not_ Valgrind I was mucking with. And the optimization used in the system library was wrong -- it stepped over the page boundary without checking. I found that it was fixed in later releases (that was all before Sun started calling their system Solaris).