4c/8629314060bfdfcdca71bf426fdb0db744079c

   1 Return-Path: <tomi.ollila@iki.fi>\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 96F7A431FBD\r
   6         for <notmuch@notmuchmail.org>; Sat,  2 Mar 2013 13:58:28 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: 0\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]\r
  12         autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id zr2dkgnBZfuE for <notmuch@notmuchmail.org>;\r
  16         Sat,  2 Mar 2013 13:58:24 -0800 (PST)\r
  17 Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34])\r
  18         by olra.theworths.org (Postfix) with ESMTP id BC36B431FAF\r
  19         for <notmuch@notmuchmail.org>; Sat,  2 Mar 2013 13:58:24 -0800 (PST)\r
  20 Received: from guru.guru-group.fi (localhost [IPv6:::1])\r
  21         by guru.guru-group.fi (Postfix) with ESMTP id 7E7AC100093;\r
  22         Sat,  2 Mar 2013 23:58:12 +0200 (EET)\r
  23 From: Tomi Ollila <tomi.ollila@iki.fi>\r
  24 To: Jani Nikula <jani@nikula.org>, notmuch@notmuchmail.org\r
  25 Subject: Re: [PATCH] config: do not overwrite symlinks when saving config file\r
  26 In-Reply-To: <1362259442-21014-1-git-send-email-jani@nikula.org>\r
  27 References: <1362259442-21014-1-git-send-email-jani@nikula.org>\r
  28 User-Agent: Notmuch/0.15.2+48~g8dcc33e (http://notmuchmail.org) Emacs/24.2.1\r
  29         (x86_64-unknown-linux-gnu)\r
  30 X-Face: HhBM'cA~<r"^Xv\KRN0P{vn'Y"Kd;zg_y3S[4)KSN~s?O\"QPoL\r
  31         $[Xv_BD:i/F$WiEWax}R(MPS`^UaptOGD`*/=@\1lKoVa9tnrg0TW?"r7aRtgk[F\r
  32         !)g;OY^,BjTbr)Np:%c_o'jj,Z\r
  33 Date: Sat, 02 Mar 2013 23:58:12 +0200\r
  34 Message-ID: <m2mwultqff.fsf@guru.guru-group.fi>\r
  35 MIME-Version: 1.0\r
  36 Content-Type: text/plain\r
  37 X-BeenThere: notmuch@notmuchmail.org\r
  38 X-Mailman-Version: 2.1.13\r
  39 Precedence: list\r
  40 List-Id: "Use and development of the notmuch mail system."\r
  41         <notmuch.notmuchmail.org>\r
  42 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  43         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  44 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  45 List-Post: <mailto:notmuch@notmuchmail.org>\r
  46 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  47 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  48         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  49 X-List-Received-Date: Sat, 02 Mar 2013 21:58:28 -0000\r
  50 \r
  51 On Sat, Mar 02 2013, Jani Nikula <jani@nikula.org> wrote:\r
  52 \r
  53 > Use realpath on the config path before writing. If that fails,\r
  54 > fallback to the previous behaviour.\r
  55 >\r
  56 > Previously 'notmuch setup' and 'notmuch config set' overwrote the\r
  57 > config file even if it was a symbolic link.\r
  58 > ---\r
  59 >  notmuch-config.c |   10 ++++++++--\r
  60 >  1 file changed, 8 insertions(+), 2 deletions(-)\r
  61 >\r
  62 > diff --git a/notmuch-config.c b/notmuch-config.c\r
  63 > index b5c2066..1e7389f 100644\r
  64 > --- a/notmuch-config.c\r
  65 > +++ b/notmuch-config.c\r
  66 > @@ -461,7 +461,7 @@ int\r
  67 >  notmuch_config_save (notmuch_config_t *config)\r
  68 >  {\r
  69 >      size_t length;\r
  70 > -    char *data;\r
  71 > +    char *data, *filename;\r
  72 >      GError *error = NULL;\r
  73 >  \r
  74 >      data = g_key_file_to_data (config->key_file, &length, NULL);\r
  75 > @@ -470,14 +470,20 @@ notmuch_config_save (notmuch_config_t *config)\r
  76 >       return 1;\r
  77 >      }\r
  78 >  \r
  79 > -    if (! g_file_set_contents (config->filename, data, length, &error)) {\r
  80 > +    /* Try not to overwrite symlinks. */\r
  81 > +    filename = realpath (config->filename, NULL);\r
  82 \r
  83 The code looks good to me. But should we handle realpath returning NULL\r
  84 differently. It should return NULL only on error. I suggest that\r
  85 when realpath returns NULL, descriptive enough error message is returned\r
  86 to the user so that the problem can be resolved and then tried again.\r
  87 \r
  88 Tomi\r
  89 \r
  90 > +\r
  91 > +    if (! g_file_set_contents (filename ? filename : config->filename,\r
  92 > +                            data, length, &error)) {\r
  93 >       fprintf (stderr, "Error saving configuration to %s: %s\n",\r
  94 >                config->filename, error->message);\r
  95 >       g_error_free (error);\r
  96 > +     free (filename);\r
  97 >       g_free (data);\r
  98 >       return 1;\r
  99 >      }\r
 100 >  \r
 101 > +    free (filename);\r
 102 >      g_free (data);\r
 103 >      return 0;\r
 104 >  }\r
 105 > -- \r
 106 > 1.7.10.4\r
 107 >\r
 108 > _______________________________________________\r
 109 > notmuch mailing list\r
 110 > notmuch@notmuchmail.org\r
 111 > http://notmuchmail.org/mailman/listinfo/notmuch\r