Hi,
I’m testing "[28972b6fc] Add support for importing statistics from remote
servers". Functionally, so far so good. I just found a piece of suspicious code
that might be an oversight in the follow-up commit aa1f93a3387:
```
/*
* Compare two RemoteAttributeMappings for sorting.
*/
static int
remattrmap_cmp(const void *v1, const void *v2)
{
const RemoteAttributeMapping *r1 = v1;
const RemoteAttributeMapping *r2 = v2;
return strncmp(r1->remote_attname, r2->remote_attname, NAMEDATALEN);
}
```
This function compares remote attribute names only up to NAMEDATALEN. But
aa1f93a3387 changed remote_attname from a NAMEDATALEN-byte array to a char
pointer, and its commit message explicitly says "the remote column name in
particular could be longer than NAMEDATALEN - 1". In that case,
remattrmap_cmp() could treat two distinct remote column names as equal for
sorting, while match_attrmap() later compares the full strings.
Also, aa1f93a3387’s commit message mentions match_attrmap(), and that function
uses strcmp(). So I guess remattrmap_cmp() should also use strcmp().
I don’t have a repro that triggers a bad result. I just want to report and
confirm whether that code should be changed. I made a local change as the
attached diff, and no test is broken with the change.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
remattrmap_cmp.diff
Description: Binary data
