unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] lib/string_map: fix return type of string_cmp
@ 2019-03-02 18:34 David Bremner
  2019-03-05  1:42 ` Matt Armstrong
  0 siblings, 1 reply; 4+ messages in thread
From: David Bremner @ 2019-03-02 18:34 UTC (permalink / raw)
  To: notmuch

I can't figure out how checking the sign of a bool ever worked. The
following program demonstrates the problem (i.e. for me it prints 1).

 #include <stdio.h>
 #include <stdbool.h>
 int main(int argc, char **argv) {
    bool x;
    x = -1;
    printf("x = %d\n", x);
 }

This seems to be mandated by the C99 standard 6.3.1.2.
---

 I'd like to get this in a new point release ASAP.

 lib/string-map.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/string-map.c b/lib/string-map.c
index ad818207..a88404c7 100644
--- a/lib/string-map.c
+++ b/lib/string-map.c
@@ -106,7 +106,7 @@ _notmuch_string_map_sort (notmuch_string_map_t *map)
     map->sorted = true;
 }
 
-static bool
+static int
 string_cmp (const char *a, const char *b, bool exact)
 {
     if (exact)
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] lib/string_map: fix return type of string_cmp
  2019-03-02 18:34 [PATCH] lib/string_map: fix return type of string_cmp David Bremner
@ 2019-03-05  1:42 ` Matt Armstrong
  2019-03-05 19:29   ` David Bremner
  0 siblings, 1 reply; 4+ messages in thread
From: Matt Armstrong @ 2019-03-05  1:42 UTC (permalink / raw)
  To: David Bremner, notmuch

David Bremner <david@tethera.net> writes:

> I can't figure out how checking the sign of a bool ever worked. The
> following program demonstrates the problem (i.e. for me it prints 1).
>
>  #include <stdio.h>
>  #include <stdbool.h>
>  int main(int argc, char **argv) {
>     bool x;
>     x = -1;
>     printf("x = %d\n", x);
>  }
>
> This seems to be mandated by the C99 standard 6.3.1.2.

Perhaps it never did work?  It looks like this function to date is used
only for properties, where I assume each message has just one or two?
With just a few elements a buggy binary search can do surprisingly well.
Don't ask me how I know this.  ;-)

Perhaps some unit tests for _notmuch_string_map would be worthwhile?


> ---
>
>  I'd like to get this in a new point release ASAP.
>
>  lib/string-map.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/string-map.c b/lib/string-map.c
> index ad818207..a88404c7 100644
> --- a/lib/string-map.c
> +++ b/lib/string-map.c
> @@ -106,7 +106,7 @@ _notmuch_string_map_sort (notmuch_string_map_t *map)
>      map->sorted = true;
>  }
>  
> -static bool
> +static int
>  string_cmp (const char *a, const char *b, bool exact)
>  {
>      if (exact)
> -- 

Looks good.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] lib/string_map: fix return type of string_cmp
  2019-03-05  1:42 ` Matt Armstrong
@ 2019-03-05 19:29   ` David Bremner
  2019-03-06  0:45     ` Matt Armstrong
  0 siblings, 1 reply; 4+ messages in thread
From: David Bremner @ 2019-03-05 19:29 UTC (permalink / raw)
  To: Matt Armstrong, notmuch

Matt Armstrong <marmstrong@google.com> writes:

> Perhaps some unit tests for _notmuch_string_map would be worthwhile?
>

Thanks for the feedback. At first I thought this would be too hard/intrusive,
since the string_map functions are not exported, but I realized
n_message_property_get is a thin wrapper on _notmuch_string_map_get, so
I added the following test. Most of the searches fail before the patch.

test_begin_subtest "testing string map binary search (via message properties)"
cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
{
   char *keys[] = {"a", "b", "c", "d", "e", NULL};
   for (int i=0; keys[i]; i++)
       EXPECT0(notmuch_message_add_property (message, keys[i], keys[i]));

   for (int i=0; keys[i]; i++) {
      EXPECT0(notmuch_message_get_property (message, keys[i], &val));
      printf("%s = %s\n", keys[i], val);
   }

   for (int i=0; keys[i]; i++)
      EXPECT0(notmuch_message_remove_property (message, keys[i], keys[i]));
}
EOF
cat <<EOF > EXPECTED
== stdout ==
a = a
b = b
c = c
d = d
e = e
== stderr ==
EOF
test_expect_equal_file EXPECTED OUTPUT

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] lib/string_map: fix return type of string_cmp
  2019-03-05 19:29   ` David Bremner
@ 2019-03-06  0:45     ` Matt Armstrong
  0 siblings, 0 replies; 4+ messages in thread
From: Matt Armstrong @ 2019-03-06  0:45 UTC (permalink / raw)
  To: David Bremner, notmuch

David Bremner <david@tethera.net> writes:

> Matt Armstrong <marmstrong@google.com> writes:
>
>> Perhaps some unit tests for _notmuch_string_map would be worthwhile?
>>
>
> Thanks for the feedback. At first I thought this would be too hard/intrusive,
> since the string_map functions are not exported, but I realized
> n_message_property_get is a thin wrapper on _notmuch_string_map_get, so
> I added the following test. Most of the searches fail before the patch.
>
> test_begin_subtest "testing string map binary search (via message properties)"
> cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
> {
>    char *keys[] = {"a", "b", "c", "d", "e", NULL};
>    for (int i=0; keys[i]; i++)
>        EXPECT0(notmuch_message_add_property (message, keys[i], keys[i]));
>
>    for (int i=0; keys[i]; i++) {
>       EXPECT0(notmuch_message_get_property (message, keys[i], &val));
>       printf("%s = %s\n", keys[i], val);
>    }
>
>    for (int i=0; keys[i]; i++)
>       EXPECT0(notmuch_message_remove_property (message, keys[i], keys[i]));
> }

Nice.  The test could even go further and verify that get_property fails
after the properties are removed.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-03-06  0:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-02 18:34 [PATCH] lib/string_map: fix return type of string_cmp David Bremner
2019-03-05  1:42 ` Matt Armstrong
2019-03-05 19:29   ` David Bremner
2019-03-06  0:45     ` Matt Armstrong

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).