Re: [PATCH v2 1/8] All access to mail files goes through the mailstore module
[notmuch-archives.git] / 80 / 49b5d0a47338e7a895d56c459f5685a1b7d897
1 Return-Path: <m.walters@qmul.ac.uk>\r
2 X-Original-To: notmuch@notmuchmail.org\r
3 Delivered-To: notmuch@notmuchmail.org\r
4 Received: from localhost (localhost [127.0.0.1])\r
5         by olra.theworths.org (Postfix) with ESMTP id 17452431FC9\r
6         for <notmuch@notmuchmail.org>; Sun,  1 Jul 2012 12:49:02 -0700 (PDT)\r
7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
8 X-Spam-Flag: NO\r
9 X-Spam-Score: -1.098\r
10 X-Spam-Level: \r
11 X-Spam-Status: No, score=-1.098 tagged_above=-999 required=5\r
12         tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001,\r
13         NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled\r
14 Received: from olra.theworths.org ([127.0.0.1])\r
15         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
16         with ESMTP id jdj8feGhOoTY for <notmuch@notmuchmail.org>;\r
17         Sun,  1 Jul 2012 12:48:58 -0700 (PDT)\r
18 Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6])\r
19         (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
20         (No client certificate requested)\r
21         by olra.theworths.org (Postfix) with ESMTPS id 9654C431FAF\r
22         for <notmuch@notmuchmail.org>; Sun,  1 Jul 2012 12:48:57 -0700 (PDT)\r
23 Received: from smtp.qmul.ac.uk ([138.37.6.40])\r
24         by mail2.qmul.ac.uk with esmtp (Exim 4.71)\r
25         (envelope-from <m.walters@qmul.ac.uk>)\r
26         id 1SlQ8V-0004Tu-D8; Sun, 01 Jul 2012 20:48:53 +0100\r
27 Received: from 94-192-233-223.zone6.bethere.co.uk ([94.192.233.223]\r
28         helo=localhost)\r
29         by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.69)\r
30         (envelope-from <m.walters@qmul.ac.uk>)\r
31         id 1SlQ8U-0002IS-NH; Sun, 01 Jul 2012 20:48:47 +0100\r
32 From: Mark Walters <markwalters1009@gmail.com>\r
33 To: Ethan Glasser-Camp <ethan.glasser.camp@gmail.com>, notmuch@notmuchmail.org\r
34 Subject: Re: [PATCH v2 1/8] All access to mail files goes through the\r
35         mailstore module\r
36 In-Reply-To: <1341160790-14525-2-git-send-email-ethan@betacantrips.com>\r
37 References:\r
38  <CAOJ+Ob0MSOez2MvD2fCgF7t32kFPk4g2+xCud88QmBLt_b5pOA@mail.gmail.com>\r
39         <1341160790-14525-1-git-send-email-ethan@betacantrips.com>\r
40         <1341160790-14525-2-git-send-email-ethan@betacantrips.com>\r
41 User-Agent: Notmuch/0.13.2+70~gb6a56e7 (http://notmuchmail.org) Emacs/23.4.1\r
42         (x86_64-pc-linux-gnu)\r
43 Date: Sun, 01 Jul 2012 20:48:41 +0100\r
44 Message-ID: <87hatr2rfq.fsf@qmul.ac.uk>\r
45 MIME-Version: 1.0\r
46 Content-Type: text/plain; charset=utf-8\r
47 Content-Transfer-Encoding: quoted-printable\r
48 X-Sender-Host-Address: 94.192.233.223\r
49 X-QM-SPAM-Info: Sender has good ham record.  :)\r
50 X-QM-Body-MD5: 16882383536cbdacb938b727e7ef7d51 (of first 20000 bytes)\r
51 X-SpamAssassin-Score: -1.8\r
52 X-SpamAssassin-SpamBar: -\r
53 X-SpamAssassin-Report: The QM spam filters have analysed this message to\r
54         determine if it is\r
55         spam. We require at least 5.0 points to mark a message as spam.\r
56         This message scored -1.8 points.\r
57         Summary of the scoring: \r
58         * -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/,\r
59         *      medium trust\r
60         *      [138.37.6.40 listed in list.dnswl.org]\r
61         * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail\r
62         provider *      (markwalters1009[at]gmail.com)\r
63         * -0.0 T_RP_MATCHES_RCVD Envelope sender domain matches handover relay\r
64         *      domain\r
65         *  0.5 AWL AWL: From: address is in the auto white-list\r
66 X-QM-Scan-Virus: ClamAV says the message is clean\r
67 Cc: Ethan Glasser-Camp <ethan@betacantrips.com>\r
68 X-BeenThere: notmuch@notmuchmail.org\r
69 X-Mailman-Version: 2.1.13\r
70 Precedence: list\r
71 List-Id: "Use and development of the notmuch mail system."\r
72         <notmuch.notmuchmail.org>\r
73 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
74         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
75 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
76 List-Post: <mailto:notmuch@notmuchmail.org>\r
77 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
78 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
79         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
80 X-List-Received-Date: Sun, 01 Jul 2012 19:49:02 -0000\r
81 \r
82 On Sun, 01 Jul 2012, Ethan Glasser-Camp <ethan.glasser.camp@gmail.com> wrot=\r
83 e:\r
84 > This commit introduces the mailstore module which provides two\r
85 > functions, notmuch_mailstore_open and notmuch_mailstore_close. These\r
86 > functions are currently just stub calls to fopen and fclose, but later\r
87 > can be made more complex in order to support mail storage systems\r
88 > where one message might not be one file.\r
89 >\r
90 > No functional changes are made, but calls to fopen and fclose are\r
91 > replaced with notmuch_mailstore_open and notmuch_mailstore_close\r
92 > wherever they are being used to access messages, but not when talking\r
93 > about real files like in notmuch-dump or notmuch-restore.\r
94 >\r
95 > Since the sha1 function notmuch_sha1_of_file is only used on messages,\r
96 > we convert it to using notmuch_mailstore_open and\r
97 > notmuch_mailstore_close, and rename it notmuch_sha1_of_message. While\r
98 > we are there, we also replace a numeric constant with its symbolic\r
99 > name BLOCK_SIZE.\r
100 >\r
101 > Signed-off-by: Ethan Glasser-Camp <ethan@betacantrips.com>\r
102 > ---\r
103 > This patch does not split notmuch_sha1_of_file the way the previous\r
104 > patch series did, because there are no other callers of\r
105 > notmuch_sha1_of_file. Instead it renames the function to\r
106 > notmuch_sha1_of_message.\r
107 >\r
108 >  lib/Makefile.local    |    1 +\r
109 >  lib/database.cc       |    2 +-\r
110 >  lib/index.cc          |    2 +-\r
111 >  lib/mailstore.c       |   34 ++++++++++++++++++++++++++++++++++\r
112 >  lib/message-file.c    |    6 +++---\r
113 >  lib/notmuch-private.h |    2 +-\r
114 >  lib/notmuch.h         |   16 ++++++++++++++++\r
115 >  lib/sha1.c            |   11 +++++------\r
116 >  mime-node.c           |    4 ++--\r
117 >  notmuch-show.c        |   12 ++++++------\r
118 >  10 files changed, 70 insertions(+), 20 deletions(-)\r
119 >  create mode 100644 lib/mailstore.c\r
120 \r
121 This patch, though large, is simple and looks obviously correct. Indeed,\r
122 given that it is really just abstracting out fopen/fclose into notmuch\r
123 specific versions it would be a good first step for any more general\r
124 mail store so it might be reasonable to accept it independently of the\r
125 rest of the series.\r
126 \r
127 Best wishes\r
128 \r
129 Mark\r
130 \r
131 >\r
132 > diff --git a/lib/Makefile.local b/lib/Makefile.local\r
133 > index 8a9aa28..cfc77bb 100644\r
134 > --- a/lib/Makefile.local\r
135 > +++ b/lib/Makefile.local\r
136 > @@ -51,6 +51,7 @@ libnotmuch_c_srcs =3D               \\r
137 >       $(dir)/filenames.c      \\r
138 >       $(dir)/string-list.c    \\r
139 >       $(dir)/libsha1.c        \\r
140 > +     $(dir)/mailstore.c      \\r
141 >       $(dir)/message-file.c   \\r
142 >       $(dir)/messages.c       \\r
143 >       $(dir)/sha1.c           \\r
144 > diff --git a/lib/database.cc b/lib/database.cc\r
145 > index 761dc1a..c035edc 100644\r
146 > --- a/lib/database.cc\r
147 > +++ b/lib/database.cc\r
148 > @@ -1773,7 +1773,7 @@ notmuch_database_add_message (notmuch_database_t *n=\r
149 otmuch,\r
150 >       if (message_id =3D=3D NULL ) {\r
151 >           /* No message-id at all, let's generate one by taking a\r
152 >            * hash over the file's contents. */\r
153 > -         char *sha1 =3D notmuch_sha1_of_file (filename);\r
154 > +         char *sha1 =3D notmuch_sha1_of_message (filename);\r
155 >=20=20\r
156 >           /* If that failed too, something is really wrong. Give up. */\r
157 >           if (sha1 =3D=3D NULL) {\r
158 > diff --git a/lib/index.cc b/lib/index.cc\r
159 > index e377732..b607e82 100644\r
160 > --- a/lib/index.cc\r
161 > +++ b/lib/index.cc\r
162 > @@ -441,7 +441,7 @@ _notmuch_message_index_file (notmuch_message_t *messa=\r
163 ge,\r
164 >       initialized =3D 1;\r
165 >      }\r
166 >=20=20\r
167 > -    file =3D fopen (filename, "r");\r
168 > +    file =3D notmuch_mailstore_open (filename);\r
169 >      if (! file) {\r
170 >       fprintf (stderr, "Error opening %s: %s\n", filename, strerror (errno));\r
171 >       ret =3D NOTMUCH_STATUS_FILE_ERROR;\r
172 > diff --git a/lib/mailstore.c b/lib/mailstore.c\r
173 > new file mode 100644\r
174 > index 0000000..48acd47\r
175 > --- /dev/null\r
176 > +++ b/lib/mailstore.c\r
177 > @@ -0,0 +1,34 @@\r
178 > +/* mailstore.c - code to access individual messages\r
179 > + *\r
180 > + * Copyright =C2=A9 2009 Carl Worth\r
181 > + *\r
182 > + * This program is free software: you can redistribute it and/or modify\r
183 > + * it under the terms of the GNU General Public License as published by\r
184 > + * the Free Software Foundation, either version 3 of the License, or\r
185 > + * (at your option) any later version.\r
186 > + *\r
187 > + * This program is distributed in the hope that it will be useful,\r
188 > + * but WITHOUT ANY WARRANTY; without even the implied warranty of\r
189 > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the\r
190 > + * GNU General Public License for more details.\r
191 > + *\r
192 > + * You should have received a copy of the GNU General Public License\r
193 > + * along with this program.  If not, see http://www.gnu.org/licenses/ .\r
194 > + *\r
195 > + * Author: Carl Worth <cworth@cworth.org>\r
196 > + */\r
197 > +#include <stdio.h>\r
198 > +\r
199 > +#include "notmuch-private.h"\r
200 > +\r
201 > +FILE *\r
202 > +notmuch_mailstore_open (const char *filename)\r
203 > +{\r
204 > +    return fopen (filename, "r");\r
205 > +}\r
206 > +\r
207 > +int\r
208 > +notmuch_mailstore_close (FILE *file)\r
209 > +{\r
210 > +    return fclose (file);\r
211 > +}\r
212 > diff --git a/lib/message-file.c b/lib/message-file.c\r
213 > index 915aba8..271389c 100644\r
214 > --- a/lib/message-file.c\r
215 > +++ b/lib/message-file.c\r
216 > @@ -86,7 +86,7 @@ _notmuch_message_file_destructor (notmuch_message_file_=\r
217 t *message)\r
218 >       g_hash_table_destroy (message->headers);\r
219 >=20=20\r
220 >      if (message->file)\r
221 > -     fclose (message->file);\r
222 > +     notmuch_mailstore_close (message->file);\r
223 >=20=20\r
224 >      return 0;\r
225 >  }\r
226 > @@ -104,7 +104,7 @@ _notmuch_message_file_open_ctx (void *ctx, const char=\r
227  *filename)\r
228 >=20=20\r
229 >      talloc_set_destructor (message, _notmuch_message_file_destructor);\r
230 >=20=20\r
231 > -    message->file =3D fopen (filename, "r");\r
232 > +    message->file =3D notmuch_mailstore_open (filename);\r
233 >      if (message->file =3D=3D NULL)\r
234 >       goto FAIL;\r
235 >=20=20\r
236 > @@ -361,7 +361,7 @@ notmuch_message_file_get_header (notmuch_message_file=\r
237 _t *message,\r
238 >      }\r
239 >=20=20\r
240 >      if (message->parsing_finished) {\r
241 > -        fclose (message->file);\r
242 > +        notmuch_mailstore_close (message->file);\r
243 >          message->file =3D NULL;\r
244 >      }\r
245 >=20=20\r
246 > diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h\r
247 > index bfb4111..ef1f994 100644\r
248 > --- a/lib/notmuch-private.h\r
249 > +++ b/lib/notmuch-private.h\r
250 > @@ -466,7 +466,7 @@ char *\r
251 >  notmuch_sha1_of_string (const char *str);\r
252 >=20=20\r
253 >  char *\r
254 > -notmuch_sha1_of_file (const char *filename);\r
255 > +notmuch_sha1_of_message (const char *filename);\r
256 >=20=20\r
257 >  /* string-list.c */\r
258 >=20=20\r
259 > diff --git a/lib/notmuch.h b/lib/notmuch.h\r
260 > index 3633bed..0ca367b 100644\r
261 > --- a/lib/notmuch.h\r
262 > +++ b/lib/notmuch.h\r
263 > @@ -1233,6 +1233,22 @@ notmuch_message_thaw (notmuch_message_t *message);\r
264 >  void\r
265 >  notmuch_message_destroy (notmuch_message_t *message);\r
266 >=20=20\r
267 > +/* Get access to the underlying data of a message.\r
268 > + *\r
269 > + * Right now, all messages are simple files in maildirs, but this is\r
270 > + * hopefully subject to change.\r
271 > + *\r
272 > + * If the returned FILE* is not NULL, be sure to call\r
273 > + * notmuch_mailstore_close when you're done with it.\r
274 > + */\r
275 > +FILE *\r
276 > +notmuch_mailstore_open (const char *filename);\r
277 > +\r
278 > +/* Release any resources associated with this file.\r
279 > + */\r
280 > +int\r
281 > +notmuch_mailstore_close (FILE *file);\r
282 > +\r
283 >  /* Is the given 'tags' iterator pointing at a valid tag.\r
284 >   *\r
285 >   * When this function returns TRUE, notmuch_tags_get will return a\r
286 > diff --git a/lib/sha1.c b/lib/sha1.c\r
287 > index cc48108..f4d213a 100644\r
288 > --- a/lib/sha1.c\r
289 > +++ b/lib/sha1.c\r
290 > @@ -65,7 +65,7 @@ notmuch_sha1_of_string (const char *str)\r
291 >  }\r
292 >=20=20\r
293 >  /* Create a hexadecimal string version of the SHA-1 digest of the\r
294 > - * contents of the named file.\r
295 > + * contents of the named message.\r
296 >   *\r
297 >   * This function returns a newly allocated string which the caller\r
298 >   * should free() when finished.\r
299 > @@ -74,7 +74,7 @@ notmuch_sha1_of_string (const char *str)\r
300 >   * file not found, etc.), this function returns NULL.\r
301 >   */\r
302 >  char *\r
303 > -notmuch_sha1_of_file (const char *filename)\r
304 > +notmuch_sha1_of_message (const char *filename)\r
305 >  {\r
306 >      FILE *file;\r
307 >  #define BLOCK_SIZE 4096\r
308 > @@ -84,14 +84,14 @@ notmuch_sha1_of_file (const char *filename)\r
309 >      unsigned char digest[SHA1_DIGEST_SIZE];\r
310 >      char *result;\r
311 >=20=20\r
312 > -    file =3D fopen (filename, "r");\r
313 > +    file =3D notmuch_mailstore_open (filename);\r
314 >      if (file =3D=3D NULL)\r
315 >       return NULL;\r
316 >=20=20\r
317 >      sha1_begin (&sha1);\r
318 >=20=20\r
319 >      while (1) {\r
320 > -     bytes_read =3D fread (block, 1, 4096, file);\r
321 > +     bytes_read =3D fread (block, 1, BLOCK_SIZE, file);\r
322 >       if (bytes_read =3D=3D 0) {\r
323 >           if (feof (file)) {\r
324 >               break;\r
325 > @@ -108,8 +108,7 @@ notmuch_sha1_of_file (const char *filename)\r
326 >=20=20\r
327 >      result =3D _hex_of_sha1_digest (digest);\r
328 >=20=20\r
329 > -    fclose (file);\r
330 > +    notmuch_mailstore_close (file);\r
331 >=20=20\r
332 >      return result;\r
333 >  }\r
334 > -\r
335 > diff --git a/mime-node.c b/mime-node.c\r
336 > index 97e8b48..a5c60d0 100644\r
337 > --- a/mime-node.c\r
338 > +++ b/mime-node.c\r
339 > @@ -49,7 +49,7 @@ _mime_node_context_free (mime_node_context_t *res)\r
340 >       g_object_unref (res->stream);\r
341 >=20=20\r
342 >      if (res->file)\r
343 > -     fclose (res->file);\r
344 > +     notmuch_mailstore_close (res->file);\r
345 >=20=20\r
346 >      return 0;\r
347 >  }\r
348 > @@ -79,7 +79,7 @@ mime_node_open (const void *ctx, notmuch_message_t *mes=\r
349 sage,\r
350 >      }\r
351 >      talloc_set_destructor (mctx, _mime_node_context_free);\r
352 >=20=20\r
353 > -    mctx->file =3D fopen (filename, "r");\r
354 > +    mctx->file =3D notmuch_mailstore_open (filename);\r
355 >      if (! mctx->file) {\r
356 >       fprintf (stderr, "Error opening %s: %s\n", filename, strerror (errno));\r
357 >       status =3D NOTMUCH_STATUS_FILE_ERROR;\r
358 > diff --git a/notmuch-show.c b/notmuch-show.c\r
359 > index 8f3c60e..daffb59 100644\r
360 > --- a/notmuch-show.c\r
361 > +++ b/notmuch-show.c\r
362 > @@ -693,7 +693,7 @@ format_part_mbox (const void *ctx, mime_node_t *node,=\r
363  unused (int indent),\r
364 >       INTERNAL_ERROR ("format_part_mbox requires a root part");\r
365 >=20=20\r
366 >      filename =3D notmuch_message_get_filename (message);\r
367 > -    file =3D fopen (filename, "r");\r
368 > +    file =3D notmuch_mailstore_open (filename);\r
369 >      if (file =3D=3D NULL) {\r
370 >       fprintf (stderr, "Failed to open %s: %s\n",\r
371 >                filename, strerror (errno));\r
372 > @@ -717,7 +717,7 @@ format_part_mbox (const void *ctx, mime_node_t *node,=\r
373  unused (int indent),\r
374 >=20=20\r
375 >      printf ("\n");\r
376 >=20=20\r
377 > -    fclose (file);\r
378 > +    notmuch_mailstore_close (file);\r
379 >=20=20\r
380 >      return NOTMUCH_STATUS_SUCCESS;\r
381 >  }\r
382 > @@ -740,7 +740,7 @@ format_part_raw (unused (const void *ctx), mime_node_=\r
383 t *node,\r
384 >           return NOTMUCH_STATUS_FILE_ERROR;\r
385 >       }\r
386 >=20=20\r
387 > -     file =3D fopen (filename, "r");\r
388 > +     file =3D notmuch_mailstore_open (filename);\r
389 >       if (file =3D=3D NULL) {\r
390 >           fprintf (stderr, "Error: Cannot open file %s: %s\n", filename, stre=\r
391 rror (errno));\r
392 >           return NOTMUCH_STATUS_FILE_ERROR;\r
393 > @@ -750,18 +750,18 @@ format_part_raw (unused (const void *ctx), mime_nod=\r
394 e_t *node,\r
395 >           size =3D fread (buf, 1, sizeof (buf), file);\r
396 >           if (ferror (file)) {\r
397 >               fprintf (stderr, "Error: Read failed from %s\n", filename);\r
398 > -             fclose (file);\r
399 > +             notmuch_mailstore_close (file);\r
400 >               return NOTMUCH_STATUS_FILE_ERROR;\r
401 >           }\r
402 >=20=20\r
403 >           if (fwrite (buf, size, 1, stdout) !=3D 1) {\r
404 >               fprintf (stderr, "Error: Write failed\n");\r
405 > -             fclose (file);\r
406 > +             notmuch_mailstore_close (file);\r
407 >               return NOTMUCH_STATUS_FILE_ERROR;\r
408 >           }\r
409 >       }\r
410 >=20=20\r
411 > -     fclose (file);\r
412 > +     notmuch_mailstore_close (file);\r
413 >       return NOTMUCH_STATUS_SUCCESS;\r
414 >      }\r
415 >=20=20\r
416 > --=20\r
417 > 1.7.9.5\r
418 >\r
419 > _______________________________________________\r
420 > notmuch mailing list\r
421 > notmuch@notmuchmail.org\r
422 > http://notmuchmail.org/mailman/listinfo/notmuch\r