Re: [PATCH] [PATCH]Btrfs-prog: uniform error handling for utils.c
You are correct, I tried to put all the error handling code in separate layer, but if there is no need to print error for each system call then it does not make sense to integrate this path. One of the userspace project on https://btrfs.wiki.kernel.org/index.php/Project_ideas#Userspace_tools_project stats that :"show meaning of various error codes, eg. for the incompatibility bits". So I thought it will be useful to have uniform error handling where it can print the error and its cause at one place and avoid the code repetition, looks like it was not the right approach. Is it possible for you to guide to implement error handling appropriately ? I would really like to work on btrfs userspace projects. Regards, Chaitanya On Tue, Oct 28, 2014 at 7:40 AM, David Sterba wrote: > Hi, > > without any explanation I can only speculate what's the purpose of this > patch. I can see it hides the basic syscalls to wrappers and prints > messages in case of an error value. > > Currently, the error codes are mostly handled and the error messages are > printed as needed, we don't want to see all of them or not exactly at > the time the error happens. > > I don't see any reason why the indirection layer is needed, the bare > syscalls are usually enough. More complexity can be hidden to wrappers > like open_file_or_dir. > > The code that's shared with kernel should stick to the kernel naming and > generic functions, for that there's kerncompat.h . The userspace code > shares the CodingStyle from kernel, but obviously has to use the > userspace API so not every function starts with btrfs_ prefix etc. > > d. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [PATCH]Btrfs-prog: uniform error handling for utils.c
Hi, without any explanation I can only speculate what's the purpose of this patch. I can see it hides the basic syscalls to wrappers and prints messages in case of an error value. Currently, the error codes are mostly handled and the error messages are printed as needed, we don't want to see all of them or not exactly at the time the error happens. I don't see any reason why the indirection layer is needed, the bare syscalls are usually enough. More complexity can be hidden to wrappers like open_file_or_dir. The code that's shared with kernel should stick to the kernel naming and generic functions, for that there's kerncompat.h . The userspace code shares the CodingStyle from kernel, but obviously has to use the userspace API so not every function starts with btrfs_ prefix etc. d. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [PATCH]Btrfs-prog: uniform error handling for utils.c
Hi,
Following path implements the uniform error handling for the utils.c
in btrfs-progs.
On Sun, Oct 12, 2014 at 2:01 PM, neo wrote:
> ---
> Makefile | 4 +-
> btrfs-syscalls.c | 180 +
> btrfs-syscalls.h | 55 +++
> kerncompat.h | 5 +-
> utils.c | 200
> +++
> 5 files changed, 337 insertions(+), 107 deletions(-)
> create mode 100644 btrfs-syscalls.c
> create mode 100644 btrfs-syscalls.h
>
> diff --git a/Makefile b/Makefile
> index 18eb944..d738f20 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -5,7 +5,7 @@ CC = gcc
> LN = ln
> AR = ar
> AM_CFLAGS = -Wall -D_FILE_OFFSET_BITS=64 -DBTRFS_FLAT_INCLUDES
> -fno-strict-aliasing -fPIC
> -CFLAGS = -g -O1 -fno-strict-aliasing
> +CFLAGS = -g -O1 -fno-strict-aliasing -rdynamic
> objects = ctree.o disk-io.o radix-tree.o extent-tree.o print-tree.o \
> root-tree.o dir-item.o file-item.o inode-item.o inode-map.o \
> extent-cache.o extent_io.o volumes.o utils.o repair.o \
> @@ -17,7 +17,7 @@ cmds_objects = cmds-subvolume.o cmds-filesystem.o
> cmds-device.o cmds-scrub.o \
>cmds-restore.o cmds-rescue.o chunk-recover.o super-recover.o \
>cmds-property.o
> libbtrfs_objects = send-stream.o send-utils.o rbtree.o btrfs-list.o crc32c.o
> \
> - uuid-tree.o utils-lib.o
> + uuid-tree.o utils-lib.o btrfs-syscalls.o
> libbtrfs_headers = send-stream.h send-utils.h send.h rbtree.h btrfs-list.h \
>crc32c.h list.h kerncompat.h radix-tree.h extent-cache.h \
>extent_io.h ioctl.h ctree.h btrfsck.h version.h
> diff --git a/btrfs-syscalls.c b/btrfs-syscalls.c
> new file mode 100644
> index 000..b4d791b
> --- /dev/null
> +++ b/btrfs-syscalls.c
> @@ -0,0 +1,180 @@
> +/***
> + * File Name : btrfs-syscalls.c
> + * Description : This file contains system call wrapper functions with
> + * uniform error handling.
> +
> **/
> +#include "btrfs-syscalls.h"
> +
> +#define BKTRACE_BUFFER_SIZE 1024
> +
> +int err_verbose = 0;
> +static void *buf[BKTRACE_BUFFER_SIZE];
> +
> +void
> +btrfs_backtrace(void)
> +{
> +int i;
> +int nptrs;
> +char **entries;
> +
> +fprintf(stderr, "Call trace:\n");
> +nptrs = backtrace(buf, BKTRACE_BUFFER_SIZE);
> +entries = backtrace_symbols(buf, nptrs);
> +if (entries == NULL) {
> +fprintf(stderr, "ERROR: backtrace_symbols\n");
> +exit(EXIT_FAILURE);
> +}
> +for (i = 0; i < nptrs; i++) {
> +if (strstr(entries[i], "btrfs_backtrace") == NULL);
> +fprintf(stderr, "\t%s\n", entries[i]);
> +}
> +free(entries);
> +}
> +
> +int
> +btrfs_open(const char *pathname, int flags)
> +{
> +int ret;
> +
> +if ((ret = open(pathname, flags)) < 0)
> +SYS_ERROR("open : %s", pathname);
> +
> +return ret;
> +}
> +
> +int
> +btrfs_close(int fd)
> +{
> +int ret;
> +
> +if ((ret = close(fd)) < 0)
> +SYS_ERROR("close :");
> +
> +return ret;
> +}
> +
> +int
> +btrfs_stat(const char *path, struct stat *buf)
> +{
> +int ret;
> +
> +if ((ret = stat(path, buf)) < 0)
> +SYS_ERROR("stat : %s", path);
> +
> +return ret;
> +}
> +
> +int
> +btrfs_lstat(const char *path, struct stat *buf)
> +{
> +int ret;
> +
> +if ((ret = lstat(path, buf)) < 0) {
> +SYS_ERROR("lstat : %s", path);
> +}
> +
> +return ret;
> +}
> +
> +int
> +btrfs_fstat(int fd, struct stat *buf)
> +{
> +int ret;
> +
> +if ((ret = fstat(fd, buf)) < 0)
> +SYS_ERROR("fstat :");
> +
> +return ret;
> +}
> +
> +void*
> +btrfs_malloc(size_t size)
> +{
> +void *p;
> +
> +if ((p = malloc(size)) == NULL) {
> +if (size != 0)
> +SYS_ERROR("malloc :");
> +}
> +
> +return p;
> +}
> +
> +void*
> +btrfs_calloc(size_t nmemb, size_t size)
> +{
> +void *p;
> +
> +if ((p = calloc(nmemb, size)) == NULL) {
> +if (size != 0)
> +SYS_ERROR("calloc :");
> +}
> +
> +return p;
> +}
> +
> +FILE*
> +btrfs_fopen(const char *path, const char *mode)
> +{
> +FILE *f;
> +
> +if ((f = fopen(path, mode)) == NULL)
> +SYS_ERROR("fopen : %s", path);
> +
> +return f;
> +}
> +
> +DIR*
> +btrfs_opendir(const char *name)
> +{
> +DIR *d;
> +
> +if ((d = opendir(name)) == NULL)
> +SYS_ERROR("opendir :");
> +
> +return d;
> +}
> +
> +int
> +btrfs_dirfd(DIR *dirp)
> +{
> +int fd;
> +
> +if ((fd = dirfd(dirp)) < 0)
> +SYS_ERROR("dirfd :");
> +
> +return fd;
> +}
> +
> +int
> +btrfs_closedir(DIR *dirp)
> +{
> +int ret;
> +
> +if ((ret = closedir(dirp)) < 0)
> +SYS_ERROR("closedir :");
> +
> +return ret;
> +}
> +
[PATCH] [PATCH]Btrfs-prog: uniform error handling for utils.c
---
Makefile | 4 +-
btrfs-syscalls.c | 180 +
btrfs-syscalls.h | 55 +++
kerncompat.h | 5 +-
utils.c | 200 +++
5 files changed, 337 insertions(+), 107 deletions(-)
create mode 100644 btrfs-syscalls.c
create mode 100644 btrfs-syscalls.h
diff --git a/Makefile b/Makefile
index 18eb944..d738f20 100644
--- a/Makefile
+++ b/Makefile
@@ -5,7 +5,7 @@ CC = gcc
LN = ln
AR = ar
AM_CFLAGS = -Wall -D_FILE_OFFSET_BITS=64 -DBTRFS_FLAT_INCLUDES
-fno-strict-aliasing -fPIC
-CFLAGS = -g -O1 -fno-strict-aliasing
+CFLAGS = -g -O1 -fno-strict-aliasing -rdynamic
objects = ctree.o disk-io.o radix-tree.o extent-tree.o print-tree.o \
root-tree.o dir-item.o file-item.o inode-item.o inode-map.o \
extent-cache.o extent_io.o volumes.o utils.o repair.o \
@@ -17,7 +17,7 @@ cmds_objects = cmds-subvolume.o cmds-filesystem.o
cmds-device.o cmds-scrub.o \
cmds-restore.o cmds-rescue.o chunk-recover.o super-recover.o \
cmds-property.o
libbtrfs_objects = send-stream.o send-utils.o rbtree.o btrfs-list.o crc32c.o \
- uuid-tree.o utils-lib.o
+ uuid-tree.o utils-lib.o btrfs-syscalls.o
libbtrfs_headers = send-stream.h send-utils.h send.h rbtree.h btrfs-list.h \
crc32c.h list.h kerncompat.h radix-tree.h extent-cache.h \
extent_io.h ioctl.h ctree.h btrfsck.h version.h
diff --git a/btrfs-syscalls.c b/btrfs-syscalls.c
new file mode 100644
index 000..b4d791b
--- /dev/null
+++ b/btrfs-syscalls.c
@@ -0,0 +1,180 @@
+/***
+ * File Name : btrfs-syscalls.c
+ * Description : This file contains system call wrapper functions with
+ * uniform error handling.
+
**/
+#include "btrfs-syscalls.h"
+
+#define BKTRACE_BUFFER_SIZE 1024
+
+int err_verbose = 0;
+static void *buf[BKTRACE_BUFFER_SIZE];
+
+void
+btrfs_backtrace(void)
+{
+int i;
+int nptrs;
+char **entries;
+
+fprintf(stderr, "Call trace:\n");
+nptrs = backtrace(buf, BKTRACE_BUFFER_SIZE);
+entries = backtrace_symbols(buf, nptrs);
+if (entries == NULL) {
+fprintf(stderr, "ERROR: backtrace_symbols\n");
+exit(EXIT_FAILURE);
+}
+for (i = 0; i < nptrs; i++) {
+if (strstr(entries[i], "btrfs_backtrace") == NULL);
+fprintf(stderr, "\t%s\n", entries[i]);
+}
+free(entries);
+}
+
+int
+btrfs_open(const char *pathname, int flags)
+{
+int ret;
+
+if ((ret = open(pathname, flags)) < 0)
+SYS_ERROR("open : %s", pathname);
+
+return ret;
+}
+
+int
+btrfs_close(int fd)
+{
+int ret;
+
+if ((ret = close(fd)) < 0)
+SYS_ERROR("close :");
+
+return ret;
+}
+
+int
+btrfs_stat(const char *path, struct stat *buf)
+{
+int ret;
+
+if ((ret = stat(path, buf)) < 0)
+SYS_ERROR("stat : %s", path);
+
+return ret;
+}
+
+int
+btrfs_lstat(const char *path, struct stat *buf)
+{
+int ret;
+
+if ((ret = lstat(path, buf)) < 0) {
+SYS_ERROR("lstat : %s", path);
+}
+
+return ret;
+}
+
+int
+btrfs_fstat(int fd, struct stat *buf)
+{
+int ret;
+
+if ((ret = fstat(fd, buf)) < 0)
+SYS_ERROR("fstat :");
+
+return ret;
+}
+
+void*
+btrfs_malloc(size_t size)
+{
+void *p;
+
+if ((p = malloc(size)) == NULL) {
+if (size != 0)
+SYS_ERROR("malloc :");
+}
+
+return p;
+}
+
+void*
+btrfs_calloc(size_t nmemb, size_t size)
+{
+void *p;
+
+if ((p = calloc(nmemb, size)) == NULL) {
+if (size != 0)
+SYS_ERROR("calloc :");
+}
+
+return p;
+}
+
+FILE*
+btrfs_fopen(const char *path, const char *mode)
+{
+FILE *f;
+
+if ((f = fopen(path, mode)) == NULL)
+SYS_ERROR("fopen : %s", path);
+
+return f;
+}
+
+DIR*
+btrfs_opendir(const char *name)
+{
+DIR *d;
+
+if ((d = opendir(name)) == NULL)
+SYS_ERROR("opendir :");
+
+return d;
+}
+
+int
+btrfs_dirfd(DIR *dirp)
+{
+int fd;
+
+if ((fd = dirfd(dirp)) < 0)
+SYS_ERROR("dirfd :");
+
+return fd;
+}
+
+int
+btrfs_closedir(DIR *dirp)
+{
+int ret;
+
+if ((ret = closedir(dirp)) < 0)
+SYS_ERROR("closedir :");
+
+return ret;
+}
+
+ssize_t
+btrfs_pwrite(int fd, const void *buf, size_t count, off_t offset)
+{
+ssize_t ret;
+
+if ((ret = pwrite(fd, buf, count, offset)) < 0)
+ SYS_ERROR("pwrite :");
+
+return ret;
+}
+
+ssize_t
+btrfs_pread(int fd, const void *buf, size_t count, off_t offset)
+{
+ssize_t ret;
+
+if ((ret = pread(fd, buf, count, offset)) < 0)
+ SYS_ERROR("pread :");
+
+return ret;
+}
diff --git a/btrfs-syscalls.h b/btrfs-syscalls.h
new file mode 100644
index 000..2c717bf
--- /dev/null
+++ b/btrfs-syscalls.h
@@
