Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Issue 24165 - Failed readf leaves File in inconsistent state #8826

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 100 additions & 53 deletions std/stdio.d
Original file line number Diff line number Diff line change
Expand Up @@ -3993,79 +3993,103 @@ enum LockType

struct LockingTextReader
{
private File _f;
private char _front;
private bool _hasChar;

this(File f)
private static struct Impl
{
import std.exception : enforce;
enforce(f.isOpen, "LockingTextReader: File must be open");
_f = f;
_FLOCK(_f._p.handle);
}
private File _f;
private char _front;
private bool _hasChar;

this(this)
{
_FLOCK(_f._p.handle);
}
this(File f)
{
import std.exception : enforce;
enforce(f.isOpen, "LockingTextReader: File must be open");
_f = f;
_FLOCK(_f._p.handle);
}

~this()
{
if (_hasChar)
ungetc(_front, cast(FILE*)_f._p.handle);
@disable this(this);

// File locking has its own reference count
if (_f.isOpen) _FUNLOCK(_f._p.handle);
}
~this()
{
if (_hasChar)
ungetc(_front, cast(FILE*)_f._p.handle);

void opAssign(LockingTextReader r)
{
import std.algorithm.mutation : swap;
swap(this, r);
}
// File locking has its own reference count
if (_f.isOpen) _FUNLOCK(_f._p.handle);
}

@property bool empty()
{
if (!_hasChar)
void opAssign(typeof(this) r)
{
import std.algorithm.mutation : swap;
swap(this, r);
}

@property bool empty()
{
if (!_f.isOpen || _f.eof)
return true;
immutable int c = _FGETC(cast(_iobuf*) _f._p.handle);
if (c == EOF)
if (!_hasChar)
{
.destroy(_f);
return true;
if (!_f.isOpen || _f.eof)
return true;
immutable int c = _FGETC(cast(_iobuf*) _f._p.handle);
if (c == EOF)
{
.destroy(_f);
return true;
}
_front = cast(char) c;
_hasChar = true;
}
_front = cast(char) c;
_hasChar = true;
return false;
}
return false;
}

@property char front()
{
if (!_hasChar)
@property char front()
{
version (assert)
if (!_hasChar)
{
import core.exception : RangeError;
if (empty)
throw new RangeError();
version (assert)
{
import core.exception : RangeError;
if (empty)
throw new RangeError();
}
else
{
empty;
}
}
else
{
return _front;
}

void popFront()
{
if (!_hasChar)
empty;
}
_hasChar = false;
}
return _front;
}

import std.typecons : SafeRefCounted, borrow;

private SafeRefCounted!Impl impl;

this(File f)
{
impl = SafeRefCounted!Impl(f);
}

@property bool empty()
{
return impl.borrow!((ref r) => r.empty);
}

@property char front()
{
return impl.borrow!((ref r) => r.front);
}

void popFront()
{
if (!_hasChar)
empty;
_hasChar = false;
impl.borrow!((ref r) { r.popFront; });
}
}

Expand Down Expand Up @@ -4143,6 +4167,29 @@ struct LockingTextReader
fr.readf("%s;%s;%s;%s\n", &nom, &fam, &nam, &ot);
}

// https://issues.dlang.org/show_bug.cgi?id=24165
@system unittest
{
// @system due to readf
static import std.file;
import std.algorithm.iteration : joiner, map;
import std.algorithm.searching : canFind;
import std.array : array;
import std.utf : byChar;

string content = "-hello";
auto deleteme = testFilename();
std.file.write(deleteme, content);
scope(exit) std.file.remove(deleteme);
File f = File(deleteme);
int n;
try f.readf("%d", n);
catch (Exception e) {}
// Data read must match what was written
char[] result = f.byLine(Yes.keepTerminator).map!byChar.joiner.array;
assert(content.canFind(result));
}

/**
* Indicates whether `T` is a file handle, i.e. the type
* is implicitly convertable to $(LREF File) or a pointer to a
Expand Down