Calvin Wan <calvinwan@xxxxxxxxxx> writes:
>> It seems that the result of applying 1-5/8 does not compile.
>
> I'll check why this is the case. I should get in the habit of compiling
> all of my patches individually.
For a 8-patch series, we should make sure that all 8 states resuting
from applying patches 1-thru-N for each N (1 <= N <= 8) builds and
tests OK for bisectability.
I often do not check that due to lack of time on the receiving end,
but for this series, I didn't understand the sudden appearance of
the object_info_options member in the code that didn't even seem to
be populated anywhere, and I noticed that the series was organized
incorrectly. Perhaps a simple rebase misordering?
>> size_index was initialized to -1 and I was expecting we can tell the
>> attribute is not used by checking (size_index < 0), but this seems
>> to make size_index 1 based, not 0 based. Intended?
>
> Yes the server returns the packet as "object_id SP size" so the first
> value is always the object_id.
I think this is to skip the object name that comes as the first item
in the response, but it would be more "pure" to keep foo_index
0-based and add the offset by whatever constant number of things
that come in front (currently, 1 for object name) near a lot closer
to where we parse and read the data, i.e. in the while() loop below.
IOW, the code that sets size_index based on the attribute query
response should not have to know how many fixed elements will come
before these attributes on the response lines. That knowledge
belongs to the code below:
> + i = 0;
> + while (packet_reader_read(&reader) == PACKET_READ_NORMAL) {
> + struct string_list object_info_values = STRING_LIST_INIT_DUP;
> +
> + string_list_split(&object_info_values, reader.line, ' ', -1);
> + if (size_index > 0) {
> + if (!strcmp(object_info_values.items[size_index].string, ""))
And here the code that uses size_index should be like
if (0 <= size_index &&
!strcmp(object_info_values.items[1 + size_index].string, ""))
...
if we wanted the logic to be more "pure" and keep foo_index 0-based.
