[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Patches for --vnlog support
From: |
Tim Rice |
Subject: |
Re: Patches for --vnlog support |
Date: |
Fri, 5 Aug 2022 23:09:07 +0000 |
Hey Dima,
I'm looking at the new _line_record_fread function and comparing it to the
wrapper functions line_record_fread and line_record_fread_vnlog_prologue. I
wonder if that could be simpler.
It seems like the `vnlog_prologue` argument to the
line_record_fread_vnlog_prologue is only used to turn on code which is already
being turned on only if the vnlog variable is already true.
I.e. this part:
```
_line_record_fread (struct /* in/out */ line_record_t* lr,
FILE *stream, char delimiter,
bool skip_comments,
bool vnlog_prologue)
{
...
if( vnlog )
{
if( vnlog_prologue )
{
```
Rather than making a new function with two wrapper functions, why don't we just
add a `if( vnlog )` toggle inside the existing `line_record_fread` function?
Or are there scenarios where vnlog will be true but vnlog_prologue could turn
out to be false?
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?
~ Tim
Re: Patches for --vnlog support, Erik Auerswald, 2022/08/07