~sircmpwn/aerc

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
1

[PATCH] notmuch: ignore blank lines in the query-map file

Details
Message ID
<20191027121756.393687-1-matt.snider@protonmail.com>
DKIM signature
pass
Download raw message
Patch: +5 -0
Hi!

This is a very small change, but as it is my first patch for aerc and first
time using `git send-email`, there may be some issues with formatting, etc. Let
me know if this is the case. Also, it's probably way too detailed for such
a minor change...

A segmentation fault occurs when using the notmuch backend and a `query-map`
file that contains blank lines. It seems that although
`worker/notmuch/worker.go:397` fails with an appropriate error,
`handleConnect()` is run anyways, and `w.db` is `nil` which causes the
segfault.

It might make sense to have an additional check here and return another err
(e.g. "notmuch database is not initialized") from `handleConnect()`. I'm not
sure if this is right, because doing that, while preventing the segfault, just
causes the interface to load (spin, spin, ...). 

Here is the console output from the segfault:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x78c0a6]

goroutine 24 [running]:
git.sr.ht/~sircmpwn/aerc/worker/notmuch/lib.(*DB).connectRO(0x0, 0xc000024238, 0x98d200)
	/home/matt/opt/aerc/worker/notmuch/lib/database.go:45 +0x26
git.sr.ht/~sircmpwn/aerc/worker/notmuch/lib.(*DB).Connect(...)
	/home/matt/opt/aerc/worker/notmuch/lib/database.go:30
git.sr.ht/~sircmpwn/aerc/worker/notmuch.(*worker).handleConnect(0xc0000b24b0, 0xc0000a3420, 0x874001, 0xc0000a0101)
	/home/matt/opt/aerc/worker/notmuch/worker.go:132 +0x33
git.sr.ht/~sircmpwn/aerc/worker/notmuch.(*worker).handleMessage(0xc0000b24b0, 0x98d000, 0xc0000a3420, 0x98d000, 0xc0000a3420)
	/home/matt/opt/aerc/worker/notmuch/worker.go:78 +0x364
git.sr.ht/~sircmpwn/aerc/worker/notmuch.(*worker).Run(0xc0000b24b0)
	/home/matt/opt/aerc/worker/notmuch/worker.go:48 +0x99
created by git.sr.ht/~sircmpwn/aerc/widgets.NewAccountView
	/home/matt/opt/aerc/widgets/account.go:74 +0x49b


---
 worker/notmuch/worker.go | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/worker/notmuch/worker.go b/worker/notmuch/worker.go
index 96adc29..685d24b 100644
--- a/worker/notmuch/worker.go
+++ b/worker/notmuch/worker.go
@@ -387,6 +387,11 @@ func (w *worker) loadQueryMap(acctConfig *config.AccountConfig) error {
 	scanner := bufio.NewScanner(f)
 	for scanner.Scan() {
 		line := scanner.Text()
+		// ignore blank lines
+		if line == "" {
+			continue
+		}
+
 		split := strings.SplitN(line, "=", 2)
 		if len(split) != 2 {
 			return fmt.Errorf("invalid line %q, want name=query", line)
-- 
2.23.0
Details
Message ID
<20191027132052.muoco66zht7b7omk@feather.localdomain>
In-Reply-To
<20191027121756.393687-1-matt.snider@protonmail.com> (view parent)
DKIM signature
pass
Download raw message
Hi Matt,
Thanks for sending in the patch, much appreciated.

Some comments below:

If you send a patch, everything you put into the mail prior to the 3 dashes
are taken as a commit message.
Now, generally you don't want discussions in there, but a concise summary of
what the patch does.

> Commit message here
> ---
>  worker/notmuch/worker.go | 5 +++++
>  1 file changed, 5 insertions(+)

Write any such comments after the --- but before the next bunch of lines (the
actual diffstats).

> Commit message here
> ---
> Some preamble comments or other remarks which are very interesting
> here.
>
>  worker/notmuch/worker.go | 5 +++++
>  1 file changed, 5 insertions(+)

The patch itself looks good.
Though you should remove the comment, it is redundant.
>  	for scanner.Scan() {
>  		line := scanner.Text()
> +		// ignore blank lines
> +		if line == "" {
> +			continue
> +		}
> +

As you probably should send in a V2 with a proper commit message, would you mind
changing the patch to also skip every line that starts with a '#' as the first character?

We might as well allow for comments while we are at it.

Keep it up!

Greetings,
Reto