Re: [PATCH 4/7] util/crypto: _n_m_crypto_potential_payload returns whether part is the payload

2019-08-01 Thread Daniel Kahn Gillmor
On Thu 2019-08-01 08:32:56 -0300, David Bremner wrote: > assert is OK, but INTERNAL_ERROR is definded in util/error_util.c, so > including that header is another sensible option. gotcha, i think i was doing these updates with not-enough-sleep. I've gone back to INTERNAL_ERROR for my internal

Re: [PATCH 4/7] util/crypto: _n_m_crypto_potential_payload returns whether part is the payload

2019-08-01 Thread David Bremner
Daniel Kahn Gillmor writes: > > Do you have more review pending on this series or should i send the > updated series to the list with these changes? > Give me a day or two, I should finish the series. d ___ notmuch mailing list

Re: [PATCH 4/7] util/crypto: _n_m_crypto_potential_payload returns whether part is the payload

2019-08-01 Thread David Bremner
Daniel Kahn Gillmor writes: > On Wed 2019-07-31 23:15:55 -0400, Daniel Kahn Gillmor wrote: >> On Wed 2019-07-31 08:57:56 -0300, David Bremner wrote: >>> what about leaving an assert or call to INTERNAL_ERROR here? I'm a bit >>> concerned this change is making the code less robust. I guess we'll

Re: [PATCH 4/7] util/crypto: _n_m_crypto_potential_payload returns whether part is the payload

2019-07-31 Thread Daniel Kahn Gillmor
On Wed 2019-07-31 23:15:55 -0400, Daniel Kahn Gillmor wrote: > On Wed 2019-07-31 08:57:56 -0300, David Bremner wrote: >> what about leaving an assert or call to INTERNAL_ERROR here? I'm a bit >> concerned this change is making the code less robust. I guess we'll see >> a segfault, if either is

Re: [PATCH 4/7] util/crypto: _n_m_crypto_potential_payload returns whether part is the payload

2019-07-31 Thread Daniel Kahn Gillmor
Hi David-- Thanks for the review! On Wed 2019-07-31 08:57:56 -0300, David Bremner wrote: > Is a bit confusing that this patch introduces a new return value, and > then ignores. Perhaps cast the calls to (void) to make it clear you are > ignoring it on purpose. hm, we ignore it only for the

Re: [PATCH 4/7] util/crypto: _n_m_crypto_potential_payload returns whether part is the payload

2019-07-31 Thread David Bremner
Daniel Kahn Gillmor writes: > Our _notmuch_message_crypto_potential_payload implementation could > only return a failure if bad arguments were passed to it. It is an > internal function, so if that happens it's an entirely internal bug > for notmuch. > > It will be more useful for this function

[PATCH 4/7] util/crypto: _n_m_crypto_potential_payload returns whether part is the payload

2019-06-24 Thread Daniel Kahn Gillmor
Our _notmuch_message_crypto_potential_payload implementation could only return a failure if bad arguments were passed to it. It is an internal function, so if that happens it's an entirely internal bug for notmuch. It will be more useful for this function to return whether or not the part is in