Re: [libvirt] [PATCH] tests: handle backspace-newline pairs in test input files

On 01/31/2011 05:24 AM, Haefliger, Juerg wrote: Thanks for tackling this! s/shorther/shorter/ Probably wise (I used just the attachment, given that the whole point of this exercise is getting rid of mailer problems, but they aren't gone until after this patch is in :) Your patch failed 'make syntax-check': TAB_in_indentation tests/testutils.c:211: /* advance the temporary buffer pointer */ maint.mk: use spaces, not TAB, for indentation in C, sh, and RNG schemas Also, I have a (minor) concern that there may be cases where we WANT to preserve a literal backslash-newline in some test files, rather than flattening it for all clients. That is, would it be better to add a new function virtTestLoadFileFlags with a flag argument that requests whether to do \\-\n flattening, and adjust callers that need it while keeping the existing virtTestLoadFile as a simple wrapper to virtTestLoadFileFlags(,0)? But as none of tests failed, that means none of our tests are affected by this, so I'm okay with the patch as-is (and in fact, that means if we add a test in the future that _does_ care about literal \\-\n, then we would would add virtTestLoadFileFlags with a non-zero to preserve literals, and not have to change any of the existing callers that get flattening by default). Mishandles any embedded NUL bytes, but again, none of our tests currently use embedded NULs, so we could add a flag in the future to force being more careful with a binary file if we have a reason to slurp in a binary file. Mishandles files that end in the two bytes backslash and newline (you back up the pointer by two, but the next fgets() returns NULL because it is at EOF without writing a NUL at the start location) - but that's trivially solvable (I squashed in tmp[len] = '\0' just after the len -= 2 line). And not optimal - it counts the string length twice (once while reading, and once at the end), but that's still O(n) and not worth the hassle of refactoring things just to keep an accurate running total while reading lines. I like it enough as is, that I'm comfortable giving: ACK with the syntax-check nit fixed So I pushed it. Thanks again for taking on my request: https://www.redhat.com/archives/libvir-list/2011-January/msg01149.html -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org