Re: Patches for --vnlog support

[Dima Kogan]

Tim Rice <trice@posteo.net> writes:

> Or are there scenarios where vnlog will be true but vnlog_prologue
> could turn out to be false?

Yep. When looking for the header, vnlog_prologue=true. After that, when
we're reading data, vnlog_prologue=false. vnlog=true being true in both
cases


> Even if we do need the vnlog_prologue toggle inside that section of
> code, there are a couple of alternative ways to do it which I might
> prefer:
>
> * We could make the existing line_record_fread variadic, so toggling
> vnlog_prologue would merely be an optional extra argument to the
> existing function.
>
> * We could use function pointers, to avoid all the `if( (!vnlog &&
> line_record_fread(...)) || ( vnlog &&
> line_record_fread_vnlog_prologue(...)))` logic which has appeared.
> Simply set the old line_record_fread as default, and override it with
> the alternative function during option processing.
>
> Thoughts?

We could do that, but I'm not clear on what problem that would solve,
and it would make stuff worse, I think. Right now you can see the logic
with your eyes. If you add indirection with function pointers, you
can't.

If you want to have a clearer separation between vnlog-specific logic
and everything else, we can have two flavors of _line_record_fread() for
the two cases, with a single if() in line_record_fread().