Have the routine it calls return a Boolean value, with "true" meaning
"keep going" and "false" meaning "stop iterating and return a failure
indication". If the callback routine never returns "false", the routine
returns "true" as a success indication.
It's not that Wireshark only supports one copy of some block options,
it's that *the pcapng specification* only supports one instance of some
block options, and it's not that wtap_block_set_*_value() fails on
non-string values, it's that the set_XXX_option_value routines currently
only support changing the value of an existing option, not adding a new
instance of an option - the latter requires the add_XXX_option_value
routine.
LINKTYPE_ERF pcap files are really ERF files inside a thin pcap wrapper
(don't even ask what a pcapng file with some or all interfaces being
LINKTYPE_ERF is...), so the time stamp comes from the ERF record, not
from the pcap packet header or pcapng block header.
The time stamp reslution for the record should reflect that, so set it
to WTAP_TSPREC_NSEC (ERF time stamps are fractional-power-of-2, not
fractional-power-of-10, so that's the best we can do).
Have them take error code and error information string arguments and,
for various failures, fill them in as "internal error" indications.
Check their return codes to see if they got an error.
Don't assume the default is correct, because there's no guarantee of
that - in fact, there's currently a guarantee that it's not, as it's
initialized to 0, which is WTAP_TSPREC_SECS.
The name of the block, in the pcapng specification is the systemd
Journal Export Block; add "export" after "journal" in various
variable/enum/define names.
Now that it's being done in common code, we don't need to do it in the
routines to read sysdig event blocks, systemd journal export blocks, or
unknown blocks.
Add in a comment to match other comments while we're at it.
Add macros to round to multiples of 2, 4, 8, 16, and 32.
Use them instead of independently defined macros.
(We don't define a general "round to a power of 2" macro to avoid the
risk of somebody passing something other than a power of 2 to it.)
Experience has shown that:
1. The current logging methods are not very reliable or practical.
A logging bitmask makes little sense as the user-facing interface (who
would want debug but not crtical messages for example?); it's
computer-friendly and user-unfriendly. More importantly the console
log level preference is initialized too late in the startup process
to be used for the logging subsystem and that fact raises a number
of annoying and hard-to-fix usability issues.
2. Coding around G_MESSAGES_DEBUG to comply with our log level mask
and not clobber the user's settings or not create unexpected log misses
is unworkable and generally follows the principle of most surprise.
The fact that G_MESSAGES_DEBUG="all" can leak to other programs using
GLib is also annoying.
3. The non-structured GLib logging API is very opinionated and lacks
configurability beyond replacing the log handler.
4. Windows GUI has some special code to attach to a console,
but it would be nice to abstract away the rest under a single
interface.
5. Using this logger seems to be noticeably faster.
Deprecate the console log level preference and extend our API to
implement a log handler in wsutil/wslog.h to provide easy-to-use,
flexible and dependable logging during all execution phases.
Log levels have a hierarchy, from most verbose to least verbose
(debug to error). When a given level is set everything above that
is also enabled.
The log level can be set with an environment variable or a command
line option (parsed as soon as possible but still later than the
environment). The default log level is "message".
Dissector logging is not included because it is not clear what log
domain they should use. An explosion to thousands of domains is
not desirable and putting everything in a single domain is probably
too coarse and noisy. For now I think it makes sense to let them do
their own thing using g_log_default_handler() and continue using the
G_MESSAGES_DEBUG mechanism with specific domains for each individual
dissector.
In the future a mechanism may be added to selectively enable these
domains at runtime while trying to avoid the problems introduced
by G_MESSAGES_DEBUG.
REC_TYPE_PACKET is 0, so if it's been initialized to 0, and never gets
overwritten, this fixes code withotu fixing a visible bug, but it should
be done anyway.
Make the variable into which we put the return value of
wtap_block_get_nth_string_option_value() a wtap_opttype_return_val, as
that's the type of the return value - it's not a boolean, it's a status
code with multiple values.
Explicitly check that value against WTAP_OPTTYPE_SUCCESS. Yes,
WTAP_OPTTYPE_SUCCESS is 0, so
if (xxx)
is equivalent to
if (xxx != WTAP_OPTTYPE_SUCCESS)
but it's better to make it explict, so it's clear that it's checking for
failure.
If the two putative number-of-records values don't match (meaning one of
them is presumably the number of records and the other one isn't - we
don't know which is the case), free up the private data structure we
allocated before returning an error.
Replace most instances of ws_debug_printf() except in
epan/dissectors and dissector plugins.
Some replacements use printf(), some use ws_debug(), and
some were removed because they were dead or judged to be
temporary.
Most of the time, the return value tells us nothing useful, as we've
already decided that we're perfectly willing to live with string
truncation. Hopefully this keeps Coverity from whining that those
routines could return an error code (NARRATOR: They don't) and thus that
we're ignoring the possibility of failure (as indicated, we've already
decided that we can live with string truncation, so truncation is *NOT*
a failure).
The secs field is a time_t, which is not necessarily 32 bits. If it's
not, casting away the upper bits, by casting to guint32, introduces a
Y2.038K bug.
Either cast to time_t or, if you're assigning a time_t to it, don't
bother with the cast.
Guy Harris