hrev50752 adds 3 changesets to branch 'master'
old head: 19d8d9fa3983e6687caeab8fe1193861a5202ab6
new head: ede463195dee8536628c54ee6a4a888c0683a83f
overview:
http://cgit.haiku-os.org/haiku/log/?qt=range&q=ede463195dee+%5E19d8d9fa3983
----------------------------------------------------------------------------
89c0b71c9a5a: BNode: Style fixes, see below for details
Remove superfluous parens
Rename result variable to bytesWritten
Sync() Unwrap ternary style fix (for consistency)
* Shouldn't this return fCStatus instead of B_FILE ERROR?
* BeBook says "returning B_OK on success and an appropriate error message
otherwise."
* Has returned B_FILE_ERROR since "it is accomplished ..."
Rename result variable to bytesWritten (again)
Remove superfluous space
explictly cast status_t to ssize_t
Remove superfluous parens, remove space, add newline
Explicitly cast status_t to ssize_t (again)
WriteAttrString() cleanup
* rename error to result
* rename sizeWritten to bytesWritten (consistency and clarity)
* check if error cede from WriteAttr and cast to status_t
* > not entirely style but functionaly same
* if length is different then written return B_FILE_ERROR
* > not a style change but a very minor functional change
* add some comments
Tiny documentation fix precede vars with \a
More style fixes
* Rename error to result
* Put parens around conditional of ternary instead of whole thing
* Compare against NULL explicitly
* Don't set fCStatus here, we're going to set it in the calling function
instead
* > note that Unset() sets fCStatus to B_NO_INIT but that's ok
80 char limit style fix
More style fixes and don't set fCStatus
* remove superfluous parens
* compare ref to NULL explicitly
* > Also don't set fCStatus here since we will do that in calling function
Unset() sets fCStatus to B_NO_INIT but that's ok
Unwrap ternary style fix (consistency)
Rename error to result (again)
6e3445098abd: BNode: Set fCStatus in SetTo() explicitly
instead of setting it in _SetTo() and then again in SetTo()
fCStatus could be set even fewer times but this is a good compromise
(logically, performance wise it is not an issue).
Update copyright, add myself to Authors
ede463195dee: BNode: CID 602323 explicitly ignore fCStatus return value in ctor
This is what it this whole push was all about
[ John Scipione <jscipione@xxxxxxxxx> ]
----------------------------------------------------------------------------
1 file changed, 70 insertions(+), 47 deletions(-)
src/kits/storage/Node.cpp | 117 +++++++++++++++++++++++++-----------------
############################################################################
Commit: 89c0b71c9a5acdbdb4543f025f64937bfbd893e3
URL: http://cgit.haiku-os.org/haiku/commit/?id=89c0b71c9a5a
Author: John Scipione <jscipione@xxxxxxxxx>
Date: Wed Dec 7 21:46:15 2016 UTC
BNode: Style fixes, see below for details
Remove superfluous parens
Rename result variable to bytesWritten
Sync() Unwrap ternary style fix (for consistency)
* Shouldn't this return fCStatus instead of B_FILE ERROR?
* BeBook says "returning B_OK on success and an appropriate error message
otherwise."
* Has returned B_FILE_ERROR since "it is accomplished ..."
Rename result variable to bytesWritten (again)
Remove superfluous space
explictly cast status_t to ssize_t
Remove superfluous parens, remove space, add newline
Explicitly cast status_t to ssize_t (again)
WriteAttrString() cleanup
* rename error to result
* rename sizeWritten to bytesWritten (consistency and clarity)
* check if error cede from WriteAttr and cast to status_t
* > not entirely style but functionaly same
* if length is different then written return B_FILE_ERROR
* > not a style change but a very minor functional change
* add some comments
Tiny documentation fix precede vars with \a
More style fixes
* Rename error to result
* Put parens around conditional of ternary instead of whole thing
* Compare against NULL explicitly
* Don't set fCStatus here, we're going to set it in the calling function instead
* > note that Unset() sets fCStatus to B_NO_INIT but that's ok
80 char limit style fix
More style fixes and don't set fCStatus
* remove superfluous parens
* compare ref to NULL explicitly
* > Also don't set fCStatus here since we will do that in calling function
Unset() sets fCStatus to B_NO_INIT but that's ok
Unwrap ternary style fix (consistency)
Rename error to result (again)
----------------------------------------------------------------------------
diff --git a/src/kits/storage/Node.cpp b/src/kits/storage/Node.cpp
index 5ecc21f..86b4015 100644
--- a/src/kits/storage/Node.cpp
+++ b/src/kits/storage/Node.cpp
@@ -60,7 +60,7 @@ node_ref::node_ref(const node_ref& other)
bool
node_ref::operator==(const node_ref& other) const
{
- return (device == other.device && node == other.node);
+ return device == other.device && node == other.node;
}
@@ -235,7 +235,10 @@ BNode::Unlock()
status_t
BNode::Sync()
{
- return (fCStatus != B_OK) ? B_FILE_ERROR : _kern_fsync(fFd);
+ if (fCStatus != B_OK)
+ return B_FILE_ERROR;
+
+ return _kern_fsync(fFd);
}
@@ -244,14 +247,15 @@ BNode::WriteAttr(const char* attr, type_code type, off_t
offset,
const void* buffer, size_t length)
{
if (fCStatus != B_OK)
- return B_FILE_ERROR;
+ return (ssize_t)B_FILE_ERROR;
if (attr == NULL || buffer == NULL)
- return B_BAD_VALUE;
+ return (ssize_t)B_BAD_VALUE;
- ssize_t result = fs_write_attr(fFd, attr, type, offset, buffer, length);
+ ssize_t bytesWritten = fs_write_attr(fFd, attr, type, offset, buffer,
+ length);
- return result < 0 ? errno : result;
+ return bytesWritten < 0 ? (ssize_t)errno : bytesWritten;
}
@@ -260,14 +264,14 @@ BNode::ReadAttr(const char* attr, type_code type, off_t
offset,
void* buffer, size_t length) const
{
if (fCStatus != B_OK)
- return B_FILE_ERROR;
+ return (ssize_t)B_FILE_ERROR;
if (attr == NULL || buffer == NULL)
- return B_BAD_VALUE;
+ return (ssize_t)B_BAD_VALUE;
- ssize_t result = fs_read_attr(fFd, attr, type, offset, buffer, length);
+ ssize_t bytesRead = fs_read_attr(fFd, attr, type, offset, buffer,
length);
- return result == -1 ? errno : result;
+ return bytesRead == -1 ? (ssize_t)errno : bytesRead;
}
@@ -297,7 +301,7 @@ BNode::GetAttrInfo(const char* name, struct attr_info*
info) const
if (name == NULL || info == NULL)
return B_BAD_VALUE;
- return fs_stat_attr(fFd, name, info) < 0 ? errno : B_OK ;
+ return fs_stat_attr(fFd, name, info) < 0 ? errno : B_OK;
}
@@ -342,16 +346,23 @@ BNode::RewindAttrs()
status_t
BNode::WriteAttrString(const char* name, const BString* data)
{
- status_t error = (!name || !data) ? B_BAD_VALUE : B_OK;
- if (error == B_OK) {
+ status_t result = (name == NULL || data == NULL) ? B_BAD_VALUE : B_OK;
+ if (result == B_OK) {
int32 length = data->Length() + 1;
- ssize_t sizeWritten = WriteAttr(name, B_STRING_TYPE, 0,
data->String(),
+ ssize_t bytesWritten = WriteAttr(name, B_STRING_TYPE, 0,
data->String(),
length);
- if (sizeWritten != length)
- error = sizeWritten;
+ if (bytesWritten < 0) {
+ // error code returned by WriteAttr()
+ result = (status_t)bytesWritten;
+ } else if (bytesWritten != length) {
+ // attribute partially written
+ result = B_FILE_ERROR;
+ }
+
+ // wrote exactly what we were supposed to, nothing more to do
}
- return error;
+ return result;
}
@@ -401,10 +412,11 @@ BNode::operator=(const BNode& node)
// Close down out current state
Unset();
+
// We have to manually dup the node, because R5::BNode::Dup()
// is not declared to be const (which IMO is retarded).
fFd = _kern_dup(node.fFd);
- fCStatus = (fFd < 0) ? B_NO_INIT : B_OK ;
+ fCStatus = fFd < 0 ? B_NO_INIT : B_OK;
return *this;
}
@@ -521,7 +533,7 @@ BNode::set_status(status_t newStatus)
/*! Initializes the BNode's file descriptor to the node referred to
- by the given FD and path combo.
+ by the given \a fd and \a path combo.
\a path must either be \c NULL, an absolute or a relative path.
In the first case, \a fd must not be \c NULL; the node it refers to will
@@ -550,8 +562,8 @@ BNode::_SetTo(int fd, const char* path, bool traverse)
{
Unset();
- status_t error = (fd >= 0 || path ? B_OK : B_BAD_VALUE);
- if (error == B_OK) {
+ status_t result = (fd >= 0 || path != NULL) ? B_OK : B_BAD_VALUE;
+ if (result == B_OK) {
int traverseFlag = (traverse ? 0 : O_NOTRAVERSE);
fFd = _kern_open(fd, path, O_RDWR | O_CLOEXEC | traverseFlag,
0);
if (fFd < B_OK && fFd != B_ENTRY_NOT_FOUND) {
@@ -559,10 +571,10 @@ BNode::_SetTo(int fd, const char* path, bool traverse)
fFd = _kern_open(fd, path, O_RDONLY | O_CLOEXEC |
traverseFlag, 0);
}
if (fFd < 0)
- error = fFd;
+ result = fFd;
}
- return fCStatus = error;
+ return result;
}
@@ -586,9 +598,9 @@ BNode::_SetTo(const entry_ref* ref, bool traverse)
{
Unset();
- status_t result = (ref ? B_OK : B_BAD_VALUE);
+ status_t result = ref != NULL ? B_OK : B_BAD_VALUE;
if (result == B_OK) {
- int traverseFlag = (traverse ? 0 : O_NOTRAVERSE);
+ int traverseFlag = traverse ? 0 : O_NOTRAVERSE;
fFd = _kern_open_entry_ref(ref->device, ref->directory,
ref->name,
O_RDWR | O_CLOEXEC | traverseFlag, 0);
if (fFd < B_OK && fFd != B_ENTRY_NOT_FOUND) {
@@ -600,7 +612,7 @@ BNode::_SetTo(const entry_ref* ref, bool traverse)
result = fFd;
}
- return fCStatus = result;
+ return result;
}
@@ -620,8 +632,7 @@ BNode::set_stat(struct stat& stat, uint32 what)
if (fCStatus != B_OK)
return B_FILE_ERROR;
- return _kern_write_stat(fFd, NULL, false, &stat, sizeof(struct stat),
- what);
+ return _kern_write_stat(fFd, NULL, false, &stat, sizeof(struct stat),
what);
}
@@ -651,9 +662,10 @@ BNode::InitAttrDir()
status_t
BNode::_GetStat(struct stat* stat) const
{
- return fCStatus != B_OK
- ? fCStatus
- : _kern_read_stat(fFd, NULL, false, stat, sizeof(struct stat));
+ if (fCStatus != B_OK)
+ return fCStatus;
+
+ return _kern_read_stat(fFd, NULL, false, stat, sizeof(struct stat));
}
@@ -661,9 +673,9 @@ status_t
BNode::_GetStat(struct stat_beos* stat) const
{
struct stat newStat;
- status_t error = _GetStat(&newStat);
- if (error != B_OK)
- return error;
+ status_t result = _GetStat(&newStat);
+ if (result != B_OK)
+ return result;
convert_to_stat_beos(&newStat, stat);
############################################################################
Commit: 6e3445098abd1d1e4cca69ba2d959fe74fe2cb1b
URL: http://cgit.haiku-os.org/haiku/commit/?id=6e3445098abd
Author: John Scipione <jscipione@xxxxxxxxx>
Date: Wed Dec 7 22:12:00 2016 UTC
BNode: Set fCStatus in SetTo() explicitly
instead of setting it in _SetTo() and then again in SetTo()
fCStatus could be set even fewer times but this is a good compromise
(logically, performance wise it is not an issue).
Update copyright, add myself to Authors
----------------------------------------------------------------------------
diff --git a/src/kits/storage/Node.cpp b/src/kits/storage/Node.cpp
index 86b4015..63f2a5b 100644
--- a/src/kits/storage/Node.cpp
+++ b/src/kits/storage/Node.cpp
@@ -1,9 +1,10 @@
/*
- * Copyright 2002-2011 Haiku, Inc. All rights reserved.
+ * Copyright 2002-2016 Haiku, Inc. All rights reserved.
* Distributed under the terms of the MIT License.
*
* Authors:
* Tyler Dauwalder
+ * John Scipione, jscipione@xxxxxxxxx
* Ingo Weinhold, bonefish@xxxxxxxxxxxx
*/
@@ -168,7 +169,9 @@ BNode::InitCheck() const
status_t
BNode::SetTo(const entry_ref* ref)
{
- return _SetTo(ref, false);
+ fCStatus = _SetTo(ref, false);
+
+ return fCStatus;
}
@@ -177,17 +180,20 @@ BNode::SetTo(const BEntry* entry)
{
if (entry == NULL) {
Unset();
- return (fCStatus = B_BAD_VALUE);
- }
+ fCStatus = B_BAD_VALUE;
+ } else
+ fCStatus = _SetTo(entry->fDirFd, entry->fName, false);
- return _SetTo(entry->fDirFd, entry->fName, false);
+ return fCStatus;
}
status_t
BNode::SetTo(const char* path)
{
- return _SetTo(-1, path, false);
+ fCStatus = _SetTo(-1, path, false);
+
+ return fCStatus;
}
@@ -197,10 +203,11 @@ BNode::SetTo(const BDirectory* dir, const char* path)
if (dir == NULL || path == NULL
|| BPrivate::Storage::is_absolute_path(path)) {
Unset();
- return (fCStatus = B_BAD_VALUE);
- }
+ fCStatus = B_BAD_VALUE;
+ } else
+ fCStatus = _SetTo(dir->fDirFd, path, false);
- return _SetTo(dir->fDirFd, path, false);
+ return fCStatus;
}
############################################################################
Revision: hrev50752
Commit: ede463195dee8536628c54ee6a4a888c0683a83f
URL: http://cgit.haiku-os.org/haiku/commit/?id=ede463195dee
Author: John Scipione <jscipione@xxxxxxxxx>
Date: Wed Dec 7 22:14:14 2016 UTC
BNode: CID 602323 explicitly ignore fCStatus return value in ctor
This is what it this whole push was all about
----------------------------------------------------------------------------
diff --git a/src/kits/storage/Node.cpp b/src/kits/storage/Node.cpp
index 63f2a5b..696567c 100644
--- a/src/kits/storage/Node.cpp
+++ b/src/kits/storage/Node.cpp
@@ -109,7 +109,8 @@ BNode::BNode(const entry_ref* ref)
fAttrFd(-1),
fCStatus(B_NO_INIT)
{
- SetTo(ref);
+ // fCStatus is set by SetTo(), ignore return value
+ (void)SetTo(ref);
}
@@ -119,7 +120,8 @@ BNode::BNode(const BEntry* entry)
fAttrFd(-1),
fCStatus(B_NO_INIT)
{
- SetTo(entry);
+ // fCStatus is set by SetTo(), ignore return value
+ (void)SetTo(entry);
}
@@ -129,7 +131,8 @@ BNode::BNode(const char* path)
fAttrFd(-1),
fCStatus(B_NO_INIT)
{
- SetTo(path);
+ // fCStatus is set by SetTo(), ignore return value
+ (void)SetTo(path);
}
@@ -139,7 +142,8 @@ BNode::BNode(const BDirectory* dir, const char* path)
fAttrFd(-1),
fCStatus(B_NO_INIT)
{
- SetTo(dir, path);
+ // fCStatus is set by SetTo(), ignore return value
+ (void)SetTo(dir, path);
}
