On 08/25/2017 06:41 PM, Martin Sebor wrote: > On 08/18/2017 04:17 AM, Martin Liška wrote: >> On 08/15/2017 02:45 PM, Martin Liška wrote: >>> Hi. >>> >>> As shown in the PR, remove_prefix function is written wrongly. It does not >>> distinguish >>> in between a node in linked list and pprefix->plist. So I decide to rewrite >>> it. >>> Apart from that, I identified discrepancy in between do_add_prefix and >>> prefix_from_string >>> where the later one appends always DIR_SEPARATOR (if missing). So I also >>> the former function. >>> And I'm adding unit tests for all functions in file-find.c. > > I know only very little about this API but from a quick glance at > the change it looks to me like it introduces an implicit assumption > that prefix points to a non-empty string. If that is in fact one > of the function's preconditions I would suggest to a) assert it > before relying on it, and b) document it. Otherwise, if the prefix > is allowed to be empty then the code below is undefined in that > case. > > Martin > > @@ -126,11 +127,22 @@ do_add_prefix (struct path_prefix *pprefix, const char > *prefix, bool first) > /* Keep track of the longest prefix. */ > > len = strlen (prefix); > + bool append_separator = !IS_DIR_SEPARATOR (prefix[len - 1]); > + if (append_separator) > + len++; > + > if (len > pprefix->max_le
Thanks Martin. I'm tending to simplify the functionality, let's see what will be discussed in the PR. Martin
