Re: Review Request 115878: File: Add a thin wrapper around different xattr implementations.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115878/#review52560 --- Ping again. 4.13 is coming soon and if this doesn't get into KDE/4.13 baloo will only build on Linux. - Raphael Kubo da Costa On March 6, 2014, 1:26 a.m., Raphael Kubo da Costa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115878/ --- (Updated March 6, 2014, 1:26 a.m.) Review request for Baloo, David Edmundson and Vishesh Handa. Repository: baloo Description --- OS X, Linux and some BSDs all support extended file attributes in a way or another, but the API differs among the operating systems. Introduce baloo_getxattr() and baloo_setxattr(), named after the OS X and Linux APIs for getting and setting extended attributes, to perform the right calls depending on the operating system. These functions accept as parameters the arguments that are common across all xattr implementations -- this means one cannot pass custom |flags| on Linux or |options| on OS X, but that should be fine for now as the code was not doing that anyway. Additionally, instead of taking raw char and void pointer, these wrappers take QStrings are arguments, which allow us to simplify the calling code and keep the QString-QByteArray-const char* conversions in a single place. Diffs - src/file/lib/autotests/filefetchjobtest.cpp c144f045f708c0541ce2eb9453c17735eda6015f src/file/lib/autotests/filemodifyjobtest.cpp 07842440934fbb20411c8d691e0088981b0e71a5 src/file/lib/baloo_xattr_p.h PRE-CREATION src/file/lib/filecustommetadata.cpp 3a01520947a324dc1674f304e44eca1ae4a19ed7 src/file/lib/xattrdetector.cpp e05984e611e8582d0e18d08f93b325381222d280 Diff: https://git.reviewboard.kde.org/r/115878/diff/ Testing --- All tests pass on FreeBSD. Thanks, Raphael Kubo da Costa Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe
Re: Review Request 115878: File: Add a thin wrapper around different xattr implementations.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115878/#review52586 --- Ship it! Looks good. I haven't tested it though, but I suppose you have. Please push into KDE/4.13, we can merge that into master. - Vishesh Handa On March 5, 2014, 11:26 p.m., Raphael Kubo da Costa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115878/ --- (Updated March 5, 2014, 11:26 p.m.) Review request for Baloo, David Edmundson and Vishesh Handa. Repository: baloo Description --- OS X, Linux and some BSDs all support extended file attributes in a way or another, but the API differs among the operating systems. Introduce baloo_getxattr() and baloo_setxattr(), named after the OS X and Linux APIs for getting and setting extended attributes, to perform the right calls depending on the operating system. These functions accept as parameters the arguments that are common across all xattr implementations -- this means one cannot pass custom |flags| on Linux or |options| on OS X, but that should be fine for now as the code was not doing that anyway. Additionally, instead of taking raw char and void pointer, these wrappers take QStrings are arguments, which allow us to simplify the calling code and keep the QString-QByteArray-const char* conversions in a single place. Diffs - src/file/lib/autotests/filefetchjobtest.cpp c144f045f708c0541ce2eb9453c17735eda6015f src/file/lib/autotests/filemodifyjobtest.cpp 07842440934fbb20411c8d691e0088981b0e71a5 src/file/lib/baloo_xattr_p.h PRE-CREATION src/file/lib/filecustommetadata.cpp 3a01520947a324dc1674f304e44eca1ae4a19ed7 src/file/lib/xattrdetector.cpp e05984e611e8582d0e18d08f93b325381222d280 Diff: https://git.reviewboard.kde.org/r/115878/diff/ Testing --- All tests pass on FreeBSD. Thanks, Raphael Kubo da Costa Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe
Re: Review Request 115878: File: Add a thin wrapper around different xattr implementations.
On March 10, 2014, 8:52 p.m., Vishesh Handa wrote: Looks good. I haven't tested it though, but I suppose you have. Please push into KDE/4.13, we can merge that into master. That's right, all the tests pass here. They were also passing on a Linux box, it's just build.kde.org that, I guess, has a filesystem that does not support extended attributes and makes some of the file autotests fail. - Raphael --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115878/#review52586 --- On March 6, 2014, 1:26 a.m., Raphael Kubo da Costa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115878/ --- (Updated March 6, 2014, 1:26 a.m.) Review request for Baloo, David Edmundson and Vishesh Handa. Repository: baloo Description --- OS X, Linux and some BSDs all support extended file attributes in a way or another, but the API differs among the operating systems. Introduce baloo_getxattr() and baloo_setxattr(), named after the OS X and Linux APIs for getting and setting extended attributes, to perform the right calls depending on the operating system. These functions accept as parameters the arguments that are common across all xattr implementations -- this means one cannot pass custom |flags| on Linux or |options| on OS X, but that should be fine for now as the code was not doing that anyway. Additionally, instead of taking raw char and void pointer, these wrappers take QStrings are arguments, which allow us to simplify the calling code and keep the QString-QByteArray-const char* conversions in a single place. Diffs - src/file/lib/autotests/filefetchjobtest.cpp c144f045f708c0541ce2eb9453c17735eda6015f src/file/lib/autotests/filemodifyjobtest.cpp 07842440934fbb20411c8d691e0088981b0e71a5 src/file/lib/baloo_xattr_p.h PRE-CREATION src/file/lib/filecustommetadata.cpp 3a01520947a324dc1674f304e44eca1ae4a19ed7 src/file/lib/xattrdetector.cpp e05984e611e8582d0e18d08f93b325381222d280 Diff: https://git.reviewboard.kde.org/r/115878/diff/ Testing --- All tests pass on FreeBSD. Thanks, Raphael Kubo da Costa Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe
Re: Review Request 115878: File: Add a thin wrapper around different xattr implementations.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115878/#review52604 --- This review has been submitted with commit 63f7804588d9357c5c31041c7d0ffd973e83113e by Raphael Kubo da Costa to branch KDE/4.13. - Commit Hook On March 5, 2014, 11:26 p.m., Raphael Kubo da Costa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115878/ --- (Updated March 5, 2014, 11:26 p.m.) Review request for Baloo, David Edmundson and Vishesh Handa. Repository: baloo Description --- OS X, Linux and some BSDs all support extended file attributes in a way or another, but the API differs among the operating systems. Introduce baloo_getxattr() and baloo_setxattr(), named after the OS X and Linux APIs for getting and setting extended attributes, to perform the right calls depending on the operating system. These functions accept as parameters the arguments that are common across all xattr implementations -- this means one cannot pass custom |flags| on Linux or |options| on OS X, but that should be fine for now as the code was not doing that anyway. Additionally, instead of taking raw char and void pointer, these wrappers take QStrings are arguments, which allow us to simplify the calling code and keep the QString-QByteArray-const char* conversions in a single place. Diffs - src/file/lib/autotests/filefetchjobtest.cpp c144f045f708c0541ce2eb9453c17735eda6015f src/file/lib/autotests/filemodifyjobtest.cpp 07842440934fbb20411c8d691e0088981b0e71a5 src/file/lib/baloo_xattr_p.h PRE-CREATION src/file/lib/filecustommetadata.cpp 3a01520947a324dc1674f304e44eca1ae4a19ed7 src/file/lib/xattrdetector.cpp e05984e611e8582d0e18d08f93b325381222d280 Diff: https://git.reviewboard.kde.org/r/115878/diff/ Testing --- All tests pass on FreeBSD. Thanks, Raphael Kubo da Costa Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe
Re: Review Request 115878: File: Add a thin wrapper around different xattr implementations.
On Feb. 19, 2014, 1:48 a.m., Thomas Lübking wrote:
src/file/lib/baloo_xattr_p.h, line 26
https://git.reviewboard.kde.org/r/115878/diff/1/?file=244875#file244875line26
I've to admit that i'm not sure how various compilers or the xattr
headers will react to this, but you might want to try:
namespace BalooXattrInternal {
#if defined(Q_OS_LINUX)
#include sys/xattr.h
...
#endif
}
namespace Baloo {
inline ssize_t getxattr(const char* path, const char* name, void*
value, size_t size)
{
#if defined(Q_OS_LINUX)
return BalooXattrInternal::getxattr(path, name, value, size);
}
using namespace BalooXattrInternal;
using Baloo::getxattr;
using Baloo::setxattr;
---
getxattr setxattr should then always be in scope and refer to the
Baloo variants (ie. the wrappers)
I'm somewhat uncomfortable with ending up having {get,set}xattr() globally like
that as the Baloo versions don't have the same arguments as the original
functions in any of the operating systems that have functions with those names.
The idea behind using baloo_function() was to follow what Qt does (ie. qt_foo()
for internal functions).
- Raphael
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115878/#review50183
---
On Feb. 19, 2014, 12:51 a.m., Raphael Kubo da Costa wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115878/
---
(Updated Feb. 19, 2014, 12:51 a.m.)
Review request for Baloo and Vishesh Handa.
Repository: baloo
Description
---
File: Add a thin wrapper around different xattr implementations.
OS X, Linux and some BSDs all support extended file attributes in a way or
another, but the API differs among the operating systems.
Introduce baloo_getxattr() and baloo_setxattr(), named after the OS X and
Linux APIs for getting and setting extended attributes, to perform the right
calls depending on the operating system.
These functions accept as parameters the arguments that are common across
all xattr implementations -- this means one cannot pass custom |flags| on
Linux or |options| on OS X, but that should be fine for now as the code was
not doing that anyway.
Diffs
-
src/file/lib/autotests/filefetchjobtest.cpp
a738c62a58d29ca092e53702d4852a48634d4315
src/file/lib/autotests/filemodifyjobtest.cpp
4ad1a6d2138d9754356a573eaf5f2487fcbe220f
src/file/lib/baloo_xattr_p.h PRE-CREATION
src/file/lib/filecustommetadata.cpp
3a01520947a324dc1674f304e44eca1ae4a19ed7
src/file/lib/xattrdetector.cpp e05984e611e8582d0e18d08f93b325381222d280
Diff: https://git.reviewboard.kde.org/r/115878/diff/
Testing
---
baloo finally builds on FreeBSD. filefetchjobtest and filemodifyjobtest fail
just like they do on Linux :-)
Thanks,
Raphael Kubo da Costa
Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe
Re: Review Request 115878: File: Add a thin wrapper around different xattr implementations.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115878/#review50490 --- src/file/lib/filecustommetadata.cpp https://git.reviewboard.kde.org/r/115878/#comment35526 why not change the path and name parameters to QByteArray/QString? and value to QByteArray. it would look cleaner if you don't have to use data()/constData() all over the place, just in the implementation. - Thomas Braxton On Feb. 18, 2014, 10:51 p.m., Raphael Kubo da Costa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115878/ --- (Updated Feb. 18, 2014, 10:51 p.m.) Review request for Baloo and Vishesh Handa. Repository: baloo Description --- File: Add a thin wrapper around different xattr implementations. OS X, Linux and some BSDs all support extended file attributes in a way or another, but the API differs among the operating systems. Introduce baloo_getxattr() and baloo_setxattr(), named after the OS X and Linux APIs for getting and setting extended attributes, to perform the right calls depending on the operating system. These functions accept as parameters the arguments that are common across all xattr implementations -- this means one cannot pass custom |flags| on Linux or |options| on OS X, but that should be fine for now as the code was not doing that anyway. Diffs - src/file/lib/autotests/filefetchjobtest.cpp a738c62a58d29ca092e53702d4852a48634d4315 src/file/lib/autotests/filemodifyjobtest.cpp 4ad1a6d2138d9754356a573eaf5f2487fcbe220f src/file/lib/baloo_xattr_p.h PRE-CREATION src/file/lib/filecustommetadata.cpp 3a01520947a324dc1674f304e44eca1ae4a19ed7 src/file/lib/xattrdetector.cpp e05984e611e8582d0e18d08f93b325381222d280 Diff: https://git.reviewboard.kde.org/r/115878/diff/ Testing --- baloo finally builds on FreeBSD. filefetchjobtest and filemodifyjobtest fail just like they do on Linux :-) Thanks, Raphael Kubo da Costa Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe
Review Request 115878: File: Add a thin wrapper around different xattr implementations.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115878/ --- Review request for Baloo and Vishesh Handa. Repository: baloo Description --- File: Add a thin wrapper around different xattr implementations. OS X, Linux and some BSDs all support extended file attributes in a way or another, but the API differs among the operating systems. Introduce baloo_getxattr() and baloo_setxattr(), named after the OS X and Linux APIs for getting and setting extended attributes, to perform the right calls depending on the operating system. These functions accept as parameters the arguments that are common across all xattr implementations -- this means one cannot pass custom |flags| on Linux or |options| on OS X, but that should be fine for now as the code was not doing that anyway. Diffs - src/file/lib/autotests/filefetchjobtest.cpp a738c62a58d29ca092e53702d4852a48634d4315 src/file/lib/autotests/filemodifyjobtest.cpp 4ad1a6d2138d9754356a573eaf5f2487fcbe220f src/file/lib/baloo_xattr_p.h PRE-CREATION src/file/lib/filecustommetadata.cpp 3a01520947a324dc1674f304e44eca1ae4a19ed7 src/file/lib/xattrdetector.cpp e05984e611e8582d0e18d08f93b325381222d280 Diff: https://git.reviewboard.kde.org/r/115878/diff/ Testing --- baloo finally builds on FreeBSD. filefetchjobtest and filemodifyjobtest fail just like they do on Linux :-) Thanks, Raphael Kubo da Costa Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe
Re: Review Request 115878: File: Add a thin wrapper around different xattr implementations.
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115878/#review50183
---
src/file/lib/baloo_xattr_p.h
https://git.reviewboard.kde.org/r/115878/#comment35288
I've to admit that i'm not sure how various compilers or the xattr headers
will react to this, but you might want to try:
namespace BalooXattrInternal {
#if defined(Q_OS_LINUX)
#include sys/xattr.h
...
#endif
}
namespace Baloo {
inline ssize_t getxattr(const char* path, const char* name, void* value,
size_t size)
{
#if defined(Q_OS_LINUX)
return BalooXattrInternal::getxattr(path, name, value, size);
}
using namespace BalooXattrInternal;
using Baloo::getxattr;
using Baloo::setxattr;
---
getxattr setxattr should then always be in scope and refer to the Baloo
variants (ie. the wrappers)
- Thomas Lübking
On Feb. 18, 2014, 10:51 p.m., Raphael Kubo da Costa wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115878/
---
(Updated Feb. 18, 2014, 10:51 p.m.)
Review request for Baloo and Vishesh Handa.
Repository: baloo
Description
---
File: Add a thin wrapper around different xattr implementations.
OS X, Linux and some BSDs all support extended file attributes in a way or
another, but the API differs among the operating systems.
Introduce baloo_getxattr() and baloo_setxattr(), named after the OS X and
Linux APIs for getting and setting extended attributes, to perform the right
calls depending on the operating system.
These functions accept as parameters the arguments that are common across
all xattr implementations -- this means one cannot pass custom |flags| on
Linux or |options| on OS X, but that should be fine for now as the code was
not doing that anyway.
Diffs
-
src/file/lib/autotests/filefetchjobtest.cpp
a738c62a58d29ca092e53702d4852a48634d4315
src/file/lib/autotests/filemodifyjobtest.cpp
4ad1a6d2138d9754356a573eaf5f2487fcbe220f
src/file/lib/baloo_xattr_p.h PRE-CREATION
src/file/lib/filecustommetadata.cpp
3a01520947a324dc1674f304e44eca1ae4a19ed7
src/file/lib/xattrdetector.cpp e05984e611e8582d0e18d08f93b325381222d280
Diff: https://git.reviewboard.kde.org/r/115878/diff/
Testing
---
baloo finally builds on FreeBSD. filefetchjobtest and filemodifyjobtest fail
just like they do on Linux :-)
Thanks,
Raphael Kubo da Costa
Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe
