Hi Ram,
Ramkumar Ramachandra wrote:
> line_buffer creates a couple of static buffers and expose an API for
> using them.
So this provides a thread-unsafe fgets() and fread() where the caller
does not have to supply a buffer. Sounds convenient.
> Taken directly
> from David Michael Barr's svn-dump-fast-export repository.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@xxxxxxxxx>
Missing From: line and sign-off.
[...]
> +char *buffer_read_line(void)
> +{
> + char *end;
style nitpick: use tabs to indent.
> + uint32_t n_read;
> +
> + if (line_len) {
> + memmove(line_buffer, &line_buffer[line_len],
> + line_buffer_len - line_len);
> + line_buffer_len -= line_len;
> + line_len = 0;
> + }
> +
> + end = memchr(line_buffer, '\n', line_buffer_len);
> + while (line_buffer_len < LINE_BUFFER_LEN - 1 &&
> + !feof(stdin) && NULL == end) {
> + n_read =
> + fread(&line_buffer[line_buffer_len], 1,
> + LINE_BUFFER_LEN - 1 - line_buffer_len,
> + stdin);
> + end = memchr(&line_buffer[line_buffer_len], '\n', n_read);
> + line_buffer_len += n_read;
> + }
Why not fgets()?
[...]
> +char *buffer_read_string(uint32_t len)
> +{
> + char *s = malloc(len + 1);
> + uint32_t offset = 0;
> + if (line_buffer_len > line_len) {
> + offset = line_buffer_len - line_len;
> + if (offset > len)
> + offset = len;
> + memcpy(s, &line_buffer[line_len], offset);
So if this buffer library is in use, all input needs to pass through
it? I would prefer to avoid that if possible.
> + line_len += offset;
> + }
> + while (offset < len && !feof(stdin)) {
> + offset += fread(&s[offset], 1, len - offset, stdin);
> + }
On error, wouldn’t this be an infinite loop? Maybe:
offset += fread(&s[offset], 1, len - offset, stdin);
if (ferror(stdin)) {
free(s);
return NULL;
}
One iteration should be sufficient, since fread loops internally.
[...]
> +void buffer_copy_bytes(uint32_t len)
> +{
> + uint32_t in, out;
> + if (line_buffer_len > line_len) {
> + in = line_buffer_len - line_len;
> + if (in > len)
> + in = len;
> + out = 0;
> + while (out < in && !ferror(stdout)) {
> + out +=
> + fwrite(&line_buffer[line_len + out], 1, in - out, stdout);
> + }
Likewise.
> + len -= in;
> + line_len += in;
> + }
> + while (len > 0 && !feof(stdin)) {
> + in = len < COPY_BUFFER_LEN ? len : COPY_BUFFER_LEN;
> + in = fread(byte_buffer, 1, in, stdin);
> + len -= in;
> + out = 0;
> + while (out < in && !ferror(stdout)) {
> + out += fwrite(&byte_buffer[out], 1, in - out, stdout);
Likewise.
Why isn’t line_buffer used here?
[...]
> +void buffer_skip_bytes(uint32_t len)
> +{
[...]
> + while (len > 0 && !feof(stdin)) {
> + in = len < COPY_BUFFER_LEN ? len : COPY_BUFFER_LEN;
> + in = fread(byte_buffer, 1, in, stdin);
> + len -= in;
> + }
Likewise.
Too bad stdio does not supply a function for this (fseek is the
closest I can find, and it does not work on unseekable files).
Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html