On 08/02/2018 07:26 AM, Bernd Edlinger wrote: > >> - if (compare_tree_int (array_size, length + 1) < 0) >> + if (nulterm) > but here you compare bytes with length which is measued un chars. > >> + *nulterm = array_elts > length; >> + else if (array_elts <= length) >> return NULL_TREE; >> >> *ptr_offset = offset; Actually in the V2 patch length is measured by element size. So I think this is a non-issue.
>> - /* Compute and store the length of the substring at OFFSET. >> + /* Compute and store the size of the substring at OFFSET. >> All offsets past the initial length refer to null strings. */ >> - if (offset <= string_length) >> - *strlen = string_length - offset; > this should be offset < string_length. > >> + if (offset <= string_size) >> + *strsize = string_size - offset; >> else >> - *strlen = 0; > this should be 1, you may access the NUL byte of "". > >> + *strsize = 0; >> } Agreed in both cases based on the defined behavior for the function (NUL is included). >> >> - const char *string = TREE_STRING_POINTER (src); >> - >> - if (string_length == 0 >> - || offset >= string_size) >> + if (string_size == 0 >> + || offset >= array_size) >> return NULL; >> >> - if (strsize) >> - { >> - /* Support even constant character arrays that aren't proper >> - NUL-terminated strings. */ >> - *strsize = string_size; >> - } >> - else if (string[string_length - 1] != '\0') > Well, this is broken for wide character strings. > but I hope we can get rid of STRING_CST which are > not explicitly null terminated. This hunk is gone in the V2 patch. So I'm not going to worry about it right now. > >> + if (!nulterm && string[string_size - 1] != '\0') >> { >> - /* Support only properly NUL-terminated strings but handle >> - consecutive strings within the same array, such as the six >> - substrings in "1\0002\0003". */ >> + /* When NULTERM is null, support only properly nul-terminated >> + strings but handle consecutive strings within the same array, >> + such as the six substrings in "1\0002\0003". Otherwise, let >> + the caller deal with non-nul-terminated arrays. */ >> return NULL; >> } >> >> - return offset <= string_length ? string + offset : ""; > this should be offset < string_size. > >> + return offset <= string_size ? string + offset : ""; Agreed. jeff