Is memcmp of struct query incorrect?

[EionRobb]

In https://github.com/vysheng/tgl/blob/master/queries.c#L105-L106 the code says

#define memcmp8(a,b) memcmp ((a), (b), 8)
DEFINE_TREE (query, struct query *, memcmp8, 0) ;

To say that a struct query is unique based on the first 8 bytes (64 bits) of the struct, however struct query is defined at https://github.com/vysheng/tgl/blob/master/queries.h#L38 to be

struct query {
  long long msg_id;
  int data_len;
  int flags;
...

Is the intention to compare msg_id's for uniqueness? because long long is not guaranteed to be exactly 64-bit, only 'at least' 64-bit in size.

Should this code instead be changed to something like

static int cmp_query(struct query *a, struct query *b) { return (a->msg_id - b->msg_id); }
DEFINE_TREE (query, struct query *, cmp_query, 0) ;

and could this be what is causing crashes in majn/telegram-purple#220

[EionRobb]

After digging into majn/telegram-purple#220 some more, I don't believe there to be a problem with memcmp of the query struct's however it does appear that there is a msg_id conflict because the tree comparison function treats msg_ids as unique, but they are only unique per DC (well, per-DC-session), as this is how generate_next_msg_id() creates them. Thus the comparison function should be something like:

static int cmp_query(struct query *a, struct query *b) {
  int ret = 0;
  ret = (a->msg_id - b->msg_id);
  return (ret ? ret : (a->session_id - b->session_id));
}

[BenWiederhake]

For the record: I just wrote a fix (the above doesn't work as-is for several reasons) and Eion is currently testing it.