This commit moves the remaining callbacks of stream/demux: pf_read/
pf_block/pf_seek/pf_readdir and pf_demux into their operations table,
aiming at unifying all the callbacks under a unique place.
This is a follow-up to the introduction of typed controls callbacks
for stream and demux.
Like for the typed controls callbacks if no operation is provided
(ie, stream_t/demux_t.ops is NULL) by a module, the legacy pf_* will be
used instead as a fallback.
The commit doesn't migrate any of modules yet.
This commit introduce a typed virtual table for operations on stream,
aiming at replacing the legacy pf_control() callback which is using
va_list. As a counterpart to the typed virtual table, typed methods
are also introduce.
The operations can be implemented by the modules directly right now
and will be used when possible. This is done to provide type safety
at every level.
When no operation is provided (ie, stream_t.ops is NULL) by a module
implementation, the legacy pf_control will be used instead as a
fallback.
The commit doesn't migrate any of access/stream_filter/… modules yet.
The commit also doesn't introduce the demux specific callbacks yet.
(matching the order of attributes in the struct)
the object is allocated with `calloc()` via `vlc_custom_create()` but
despite this explicit allocation is wanted.
This reverts commit 830ea36e70.
as requested in follow-up discussion in !1104.
(actually this is now a partial revert - the change to the return line has
been left in place per review request in !1193).
`vlc_custom_create()` uses `calloc()` to allocate the memory, so
initialising all of the attributes individually to null/0/false is
unnecessary.
the initialisation was not even complete, missing all of these:
- s->psz_name
- s->psz_location
- s->b_preparsing
- s->out
- s->pf_demux
if in "any" mode and falling back to an extension based match, and if the
`asprintf()` call fails to create the extension based shortcut search
string, the consensus is that it should give up rather than proceed to try
an "any" based demux module load.
note that the `modbuf = NULL` statement was just resetting the undefined
state of the variable from the failed `asprintf()` call to ensure that the
later `free(modbuf)` was valid. it is not needed now that we go down the
error path.
note, the init of `s->psz_filepath` in `vlc_stream_CustomNew()` is not
strictly necessary to avoid `free()` on an initialised attribute, since
`calloc()` is actually involved in the object creation - much of the
initialisation done in `vlc_stream_CustomNew()` is actually redundant.
This reduces the number of buffer reallocations performed for long
lines, turning the function's complexity from quadratic to linear. The
line length limit remains unchanged.
Upon error, data already consumed from the stream could be discarded
and permanently lost for the caller. This was just bad, and notably
precluded recovery from errors. Instead, data is now preserved within
peek buffers until a line can be successfully returned.
Peak memory usage remains almost identical, while the number of
allocations is halved for long lines. For UTF-16 streams, memory usage
is reduced as needless buffer allocations and copies are eliminated.
Lone-byte incomplete UTF-16 sequences before EOF, in some cases such as
a final line consisting only of it, would never get actually consumed
from the stream, preventing it from ever properly reaching EOF.
This also avoids flooding the logs with one warning per stream line
towards the end of the stream, and then printing an unspecific
conversion error: those are replaced by one clear and explicit error
message.
The only errors after which this was called were memory allocation
errors, lines too long, or failing to open the handle itself, so
obviously there is no reason to want to close it there; it already gets
closed in the proper place when the stream is destroyed.
Even worse, it left the handle missing if vlc_stream_ReadLine() was
called again, and would result in text conversion constantly failing and
no output getting returned anymore, rendering the rest of the stream
unusable through this API and precluding any error recovery.
Objects have one strong reference held by their "owner", and zero or
more weak references generated by vlc_object_hold() et al. This
provides a separate function to remove the strong reference than
vlc_object_release() to remove weak ones.
With this sole change, this is really only an annotation though.
The conversion from UTF-16 to UTF-8 could (until a few days ago) fail
before the end. In that case, the appended nul terminator would not be
converted. A nul terminator is anyway always appended after conversion
and trimming.
If conversion failed on the first character, that nul terminator would
be written at a negative offset, leading to heap buffer "underflow" and
memory corruption. This was fixed but lead to a mismatch in the value of
i_lines depending on the character width.
This change removes the useless pre-conversion nul terminator, and thus
makes trimming work again with single byte character width work. This
fixes reading text files formatted with MS-DOS line endings.
Do not erase the last converted byte. This bug has apparently existed
ever since UTF-16 support was added.
If the conversion fails, this bug resulted in a heap underflow (writing
zero right before the beginning of the buffer).
access_t.pf_read did not expect a NULL output pointer before the merge
of access_t and stream_t. For files, this caused an EFAULT error, but
for other input types, it would likely crash.
The function does not open MRLs, as correctly described in its
documentation, as such it is rather unfortunate that its name uses MRL
instead of URL (especially given that it cannot handle MRLs).
These changes are simply renaming all occurrences of the function, so
that the behavior of the function is properly reflected by its name.
Signed-off-by: Thomas Guillem <thomas@gllm.fr>
The result of a read operation is a signed size_t, and cannot be
negative (except on error). Thus reading more than SSIZE_MAX bytes at
once is not well defined.
(Note: POSIX marks it as implementation-defined, and we cannot rely on
much given the different implementations.)
In practice, this is not really a limitation for regular reads as
allocating a contiguous output buffer of more than SSIZE_MAX bytes is
essentially impossible. It can however be a problem when skipping data
(buffer pointer is NULL), especially on 32-bits platforms.
To skip such large amount of data, seeking is recommended instead,
e.g.:
vlc_stream_Seek(s, vlc_stream_Tell() + skip);
instead of:
vlc_stream_Read(s, NULL, skip);
The if-statement in question prevents stream_ReadLine to work if invoked
with a block-based stream, as such the condition has now been altered to
only return immediately if the source stream does not have neither of
pf_read and pf_block.
Signed-off-by: Rémi Denis-Courmont <remi@remlab.net>
Loïc Branstett