labath added a comment.
In https://reviews.llvm.org/D25783#574873, @zturner wrote:
> In https://reviews.llvm.org/D25783#574860, @labath wrote:
>
> > > This can happen with any number of bytes and at any time. `write`,
> > > `read`, and all other related functions will return a non-negative valu
Actually ignore that last comment , the behavior is the same as before, but
linux will still behave differently than other platforms
On Wed, Oct 19, 2016 at 3:17 PM Zachary Turner wrote:
> zturner added a comment.
>
> Incidentally, this patch actually makes all platforms behave consistently
> whe
zturner added a comment.
Incidentally, this patch actually makes all platforms behave consistently when
the `Write` overload with offset is used with `O_APPEND`, so there's probably
some value in having that consistency.
https://reviews.llvm.org/D25783
__
zturner added a comment.
In https://reviews.llvm.org/D25783#574860, @labath wrote:
> > This can happen with any number of bytes and at any time. `write`, `read`,
> > and all other related functions will return a non-negative value indicating
> > the number of bytes successfully read/written, w
labath added a comment.
> This can happen with any number of bytes and at any time. `write`, `read`,
> and all other related functions will return a non-negative value indicating
> the number of bytes successfully read/written, which will be less than the
> number requested.
Except if fd refe
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
Ok, the lseek calls make sense then.
https://reviews.llvm.org/D25783
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.l
zturner added inline comments.
Comment at: source/Host/common/File.cpp:405
-if (bytes_read == 0) {
- if (::feof(m_stream))
-error.SetErrorString("feof");
- else if (::ferror(m_stream))
-error.SetErrorString("ferror");
- num_bytes = 0;
-} e
zturner added a comment.
Also you are right that I misspoke about the append case. But still, I just
think that if writing to the same file from multiple processes is something we
care about, we should support it "for real" instead of just pretending to.
That means some kind of cross-process
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
Just a few questions on why we are calling lseek in the read and write
functions. See inlined comments.
Comment at: source/Host/common/File.cpp:405
-if (
zturner added a comment.
In https://reviews.llvm.org/D25783#574699, @labath wrote:
> In https://reviews.llvm.org/D25783#574684, @zturner wrote:
>
> > There are many other problems with this code if we want to deal with
> > atomicity. For example, the whole point of this patch was to handle shor
labath added a comment.
In https://reviews.llvm.org/D25783#574684, @zturner wrote:
> There are many other problems with this code if we want to deal with
> atomicity. For example, the whole point of this patch was to handle short
> reads and writes. Well, if you have a short read or a write,
zturner added a comment.
Also, for the record, if you specify the threadsafe logging option, it already
does put this in a mutex, so there should be no issue.
https://reviews.llvm.org/D25783
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
zturner added a comment.
In https://reviews.llvm.org/D25783#574679, @labath wrote:
> I am not sure the "append" thing is actually a "fix". I consider it more like
> a feature of the append mode. It's also nice that it guarantees atomicity of
> writes even if two processes are writing to the sam
labath added a comment.
I am not sure the "append" thing is actually a "fix". I consider it more like a
feature of the append mode. It's also nice that it guarantees atomicity of
writes even if two processes are writing to the same file (very useful for
logging, although I'm not sure if that go
zturner created this revision.
zturner added reviewers: clayborg, beanz, EugeneBi.
zturner added a subscriber: lldb-commits.
The original motivation for this came from https://reviews.llvm.org/D25712, in
which Eugene pointed out that `File::Read()` does not correctly handle short
reads. However
15 matches
Mail list logo
