Hi, On 21/09/2020 09:50, Gert Doering wrote: > Hi, > > On Mon, Sep 21, 2020 at 09:22:38AM +0200, Antonio Quartulli wrote: >> Sorry for not chiming in earlier, but honestly I believe your other >> option would be "cleaner". The other option being "return int instead of >> bool, where the returned value is the number of lines of the inline'd >> material. >> >> If 0, we know there was no inline'd material, thus we can treat that >> result as "not inline'd". > > We could do that, indeed. I was torn between both variants - this one > is a bit more intrusive (because it adds a new argument to so many > functions), but on the other hand, the bool stays a bool and is not > magically overloaded - which would make the call to add_option() > either "even longer", or if you rely on magic "we just pass the integer > to a bool-expecting function" much harder to understand. > >> Anyway, this is just my opinion. If you believe this is a better >> approach, I am fine with it too. > > I wasn't sure. In the end, this one didn't feel so bad, so this is > what we have. > > Maybe I'll do the other one as well, for fun, and to compare :-) > >>> @@ -4692,7 +4696,7 @@ check_inline_file(struct in_src *is, char *p[], >>> struct gc_arena *gc) >>> p[0] = string_alloc(arg + 1, gc); >>> close_tag = alloc_buf(strlen(p[0]) + 4); >>> buf_printf(&close_tag, "</%s>", p[0]); >>> - p[1] = read_inline_file(is, BSTR(&close_tag), gc); >>> + p[1] = read_inline_file(is, BSTR(&close_tag), num_lines, gc); >> >> am I wrong or here we should add 1 to num_lines to include the opening >> tag that was parsed *before* entering read_inline_file()? > > The opening tag is parsed as "regular" line in read_config_file(), and as > such, the "++line_num" in there applies. > > (I have tested this :-) - nothing more embarrassing than a patch to fix > line numbers actually still printing wrong line numbers) >
Ok, then for me this patch is good to go. FTR on IRC we discussed the possibility of using the "other approach". Gert may give it a try and decide if he wants to send that version to the ml as well. In any case, this patch gets my blessing: Acked-by: Antonio Quartulli <a...@unstable.cc> -- Antonio Quartulli _______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
