[PATCH] lib: another iterator-temporary/stale-pointer bug
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 at pamparam > [2]: id:20141226230655.GA41992 at 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, , 10); > -- > 2.1.3 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] lib: another iterator-temporary/stale-pointer bug
On Sun, 28 Dec 2014, David Bremner da...@tethera.net 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 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] lib: another iterator-temporary/stale-pointer bug
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 at pamparam [2]: id:20141226230655.GA41992 at 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; /* 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, , 10); -- 2.1.3
[PATCH] lib: another iterator-temporary/stale-pointer bug
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; /* 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
