From 7c8d15bb0cd2652f2a3c63c87a9e965586ac0d3a Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Thu, 1 Jan 2015 16:49:31 +0200 Subject: [PATCH] Re: [PATCH] lib: another iterator-temporary/stale-pointer bug --- d8/494679b3b35ebd43aac72c02e56298ce056eff | 122 ++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 d8/494679b3b35ebd43aac72c02e56298ce056eff diff --git a/d8/494679b3b35ebd43aac72c02e56298ce056eff b/d8/494679b3b35ebd43aac72c02e56298ce056eff new file mode 100644 index 000000000..adc3be37f --- /dev/null +++ b/d8/494679b3b35ebd43aac72c02e56298ce056eff @@ -0,0 +1,122 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 27DD5431FC7 + for ; Thu, 1 Jan 2015 06:49:28 -0800 (PST) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -0.7 +X-Spam-Level: +X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 + tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id hL47YPpuMLcH for ; + Thu, 1 Jan 2015 06:49:24 -0800 (PST) +Received: from mail-wg0-f50.google.com (mail-wg0-f50.google.com + [74.125.82.50]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client + certificate requested) by olra.theworths.org (Postfix) with ESMTPS id + A95F6431FC0 for ; Thu, 1 Jan 2015 06:49:24 -0800 + (PST) +Received: by mail-wg0-f50.google.com with SMTP id a1so23207072wgh.37 + for ; Thu, 01 Jan 2015 06:49:23 -0800 (PST) +X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; + d=1e100.net; s=20130820; + h=x-gm-message-state:from:to:subject:in-reply-to:references + :user-agent:date:message-id:mime-version:content-type; + bh=bF86MTEfW/5kYtavRnJ/Q9Fskg1J7YqeBSZUAiOixcE=; + b=VKLMMSPRl4OskEnpgBUlSTYWOW68FBiR06FS0lT29cR/ilzPTSAhIVGeKY6SuIU61B + KXe5LHodDOcnnKQ0pqvXahzzhrI7TG+AtEnobcPmtSvRmPzN5wQ+JAT6hMVDTepjjgOf + YrRazXtJtlj6q1FWpqpjCIcMuSB1aDV6WIkgKuNuYKu6xa0IokucOpbM2bYN1sxPTgXx + U+gAQapabYJz77WvU8Tb8yRUxZ+SS765SLbydnF18nyk9Ut1cxlpp+/XjJOJoWwIbCrs + OioFSiOgWkstkunlLDca84yeJK9HleVYeswLdB+E6KLUXbzLUk6qE0lea9bfKu5HDHPs + WouQ== +X-Gm-Message-State: + ALoCoQlxftuQgyoJWM+DLZ6rAFPtBVgoIzz3S4hXl+T+W352tt2ABXwFU5QtFk21shnf5sKgDwd+ +X-Received: by 10.180.20.242 with SMTP id q18mr122513203wie.80.1420123763596; + Thu, 01 Jan 2015 06:49:23 -0800 (PST) +Received: from localhost (dsl-hkibrasgw2-58c371-91.dhcp.inet.fi. + [88.195.113.91]) + by mx.google.com with ESMTPSA id u3sm50666470wiw.24.2015.01.01.06.49.22 + (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); + Thu, 01 Jan 2015 06:49:23 -0800 (PST) +From: Jani Nikula +To: David Bremner , notmuch@notmuchmail.org +Subject: Re: [PATCH] lib: another iterator-temporary/stale-pointer bug +In-Reply-To: <1419763508-11902-1-git-send-email-david@tethera.net> +References: <20141226230655.GA41992@pamparam> + <1419763508-11902-1-git-send-email-david@tethera.net> +User-Agent: Notmuch/0.19+3~g7585e8c (http://notmuchmail.org) Emacs/24.4.1 + (x86_64-pc-linux-gnu) +Date: Thu, 01 Jan 2015 16:49:31 +0200 +Message-ID: <87wq56wo78.fsf@nikula.org> +MIME-Version: 1.0 +Content-Type: text/plain +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Thu, 01 Jan 2015 14:49:28 -0000 + +On Sun, 28 Dec 2014, David Bremner wrote: +> Tamas Szakaly points out [1] that the bug fixed in 51b073c still +> exists in at least one place. This change follows the suggestion of +> [2] and creates a block scope temporary std::string to avoid the rules +> of iterators temporaries. +> +> [1]: id:20141226113755.GA64154@pamparam +> [2]: id:20141226230655.GA41992@pamparam +> --- +> +> I decided to take a more minimalist approach than [2]. In particular +> using "direntry" for two different things seemed slightly trickier +> than necessary, for no obvious performance gain (calling .c_str() +> should be cheap). +> +> lib/message.cc | 5 +++-- +> 1 file changed, 3 insertions(+), 2 deletions(-) +> +> diff --git a/lib/message.cc b/lib/message.cc +> index a7a13cc..bacb4d4 100644 +> --- a/lib/message.cc +> +++ b/lib/message.cc +> @@ -641,15 +641,16 @@ _notmuch_message_add_directory_terms (void *ctx, notmuch_message_t *message) +> unsigned int directory_id; +> const char *direntry, *directory; +> char *colon; +> + const std::string term = *i; + +You could use a reference here like in [2]. Either way, LGTM. + +Jani. + +> +> /* Terminate loop at first term without desired prefix. */ +> - if (strncmp ((*i).c_str (), direntry_prefix, direntry_prefix_len)) +> + if (strncmp (term.c_str (), direntry_prefix, direntry_prefix_len)) +> break; +> +> /* Indicate that there are filenames remaining. */ +> status = NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID; +> +> - direntry = (*i).c_str (); +> + direntry = term.c_str (); +> direntry += direntry_prefix_len; +> +> directory_id = strtol (direntry, &colon, 10); +> -- +> 2.1.3 +> +> _______________________________________________ +> notmuch mailing list +> notmuch@notmuchmail.org +> http://notmuchmail.org/mailman/listinfo/notmuch -- 2.26.2