Re: [libvirt] [PATCH v2 2/6] logical: Fix parsing of the 'devices' field lvs output

> That's not true, regex is greedy and it tries to match as much as possible: > > https://regex101.com/r/lS9tZ5/1 > > Like I said, the regex is more robust and parse the string correctly event with > comma as part of a device name, for example "/dev/sda,1". > So do you want the regex back in? Doesn't really matter to me either way. I am curious though about what construct is "/dev/sda,1" - that is, how is it used?

>>>> - >>>> - vol->source.extents[vol->source.nextent].start = offset * size; >>>> - vol->source.extents[vol->source.nextent].end = (offset * size) + length; >>>> - vol->source.nextent++; >>>> + vol->source.extents[i].start = offset * size; >>>> + vol->source.extents[i].end = (offset * size) + length; >>> >>> [1] ... there you would call VIR_APPEND_ELEMENT(). >>> >> >> I don't find the whole VIR_APPEND_ELEMENT option as clean... But I'll >> go with it ... >> >> Usage will require a change to src/conf/storage_conf.h in order to >> change 'nextent' to a 'size_t' (from an 'int') - compiler complains >> otherwise. >> >> Then remove the VIR_REALLOC_N >> >> Add a memset(&extent, 0, sizeof(extent)); prior to the for loop and one >> at the end of the for loop after the successful VIR_APPEND_ELEMENT > > You don't need to add one after the successful VIR_APPEND_ELEMENT, it does that > for us. There is a different version called VIR_APPEND_ELEMENT_COPY to not > clean the original element. > > In this case, where we know the number of elements, we can pre-allocate the > array and use VIR_APPEND_ELEMENT_INSERT which requires the array to be > allocated. > In which case VIR_REALLOC_N of some known sized array and then accessing each element isn't that much different. Since the more common case is "1" extent the difference between the two is nothing, especially since in the long run the APPEND macros call virReallocN. I perhaps would feel different if I didn't know the value of 'nextents' or this code was adding 1 extent at a time... Ironically the two places that use VIR_APPEND_ELEMENT_INPLACE have callers that will use VIR_REALLOC_N (qemuDomainAttachRNGDevice and in a much more convoluted path via virDomainChrPreAlloc).