[SSSD] [PATCH 1/2] Reuse sss_open_cloexec at other places in code.
Lukas Slebodnik
lslebodn at redhat.com
Mon Mar 11 15:58:22 UTC 2013
On (11/03/13 15:26), Ondrej Kos wrote:
>On 03/11/2013 11:42 AM, Lukas Slebodnik wrote:
>>On (11/03/13 10:48), Ondrej Kos wrote:
>>>On 03/11/2013 10:22 AM, Lukas Slebodnik wrote:
>>>>diff --git a/src/util/util.h b/src/util/util.h
>>>>index 8afb3bc964dfc7ea5debf8127b7c6bac08d11f42..2adc8cb4180ae802afee63d084c3d1d67f273862 100644
>>>>--- a/src/util/util.h
>>>>+++ b/src/util/util.h
>>>>@@ -584,6 +584,9 @@ errno_t sssd_domain_init(TALLOC_CTX *mem_ctx,
>>>> /* from util_lock.c */
>>>> errno_t sss_br_lock_file(int fd, size_t start, size_t len,
>>>> int num_tries, useconds_t wait);
>>>>+/* from compact.c */
>>>
>>>if the file is renamed to io.c, please rewrite also this comment to
>>>avoid confusion
>>>
>>>>+int sss_open_cloexec(const char *pathname, int flags, int *ret);
>>>>+int sss_openat_cloexec(int dir_fd, const char *pathname, int flags, int *ret);
>>>
>>>
>>>--
>>>Ondrej Kos
>>>Associate Software Engineer
>>>Identity Management
>>>Red Hat Czech
>>>
>>>phone: +420-532-294-558
>>>cell: +420-736-417-909
>>>ext: 82-62558
>>>loc: 1013 Brno 1 office
>>>irc: okos @ #sssd #brno
>>
>>Thank you for the notice, patches attached.
>>
>
>there are left unused variables after application of your patches,
>remove these and I'll ack them
>
>src/providers/ldap/sdap_child_helpers.c: In function 'setup_child':
>src/providers/ldap/sdap_child_helpers.c:456:14: warning: unused
>variable 'v' [-Wunused-variable]
>
>src/providers/krb5/krb5_init_shared.c: In function 'krb5_child_init':
>src/providers/krb5/krb5_init_shared.c:33:14: warning: unused variable
>'v' [-Wunused-variable]
>
>--
>Ondrej Kos
I hope it is the last time.
LS
-------------- next part --------------
>From 421cbab1a9e406e300e2b7f843ba53a4f0f62cd4 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Fri, 8 Mar 2013 14:26:42 +0100
Subject: [PATCH 1/2] Reuse sss_open_cloexec at other places in code.
Functions open_cloexec and openat_cloexec were renamed with prefix
"sss_" and moved to separete file. Replacing duplicated code of
function sss_open_cloexec everywhere in the source code.
https://fedorahosted.org/sssd/ticket/1794
---
Makefile.am | 6 ++-
src/sss_client/nss_mc_common.c | 19 +--------
src/tools/files.c | 70 ++------------------------------
src/util/io.c | 90 ++++++++++++++++++++++++++++++++++++++++++
src/util/util.h | 3 ++
5 files changed, 103 insertions(+), 85 deletions(-)
create mode 100644 src/util/io.c
diff --git a/Makefile.am b/Makefile.am
index c66e25525c2a652681eb8ff2cd607d368d59bc81..13498efabb901031130126e6739ad4c9fbcfe7f2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -527,7 +527,8 @@ libsss_util_la_SOURCES = \
src/util/sss_selinux.c \
src/util/domain_info_utils.c \
src/util/util_lock.c \
- src/util/util_errors.c
+ src/util/util_errors.c \
+ src/util/io.c
libsss_util_la_LIBADD = \
$(SSSD_LIBS) \
$(UNICODE_LIBS) \
@@ -921,7 +922,8 @@ files_tests_CFLAGS = \
files_tests_LDADD = \
libsss_debug.la \
$(FILES_TESTS_LIBS) \
- libsss_test_common.la
+ libsss_test_common.la \
+ libsss_util.la
SSSD_RESOLV_TESTS_OBJ = \
$(SSSD_RESOLV_OBJ)
diff --git a/src/sss_client/nss_mc_common.c b/src/sss_client/nss_mc_common.c
index 78490bef0edc99efbd1b953c35183921d5b02b64..66442b7174c38fa4e486b979e8067b1b4dfe448d 100644
--- a/src/sss_client/nss_mc_common.c
+++ b/src/sss_client/nss_mc_common.c
@@ -31,6 +31,7 @@
#include <string.h>
#include <stdlib.h>
#include "nss_mc.h"
+#include "util/util.h"
/* FIXME: hook up to library destructor to avoid leaks */
/* FIXME: temporarily open passwd file on our own, later we will probably
@@ -100,12 +101,6 @@ errno_t sss_nss_check_header(struct sss_cli_mc_ctx *ctx)
return 0;
}
-#ifdef O_CLOEXEC
-#define SSS_MC_OPEN_FLAGS O_RDONLY|O_CLOEXEC
-#else
-#define SSS_MC_OPEN_FLAGS O_RDONLY
-#endif
-
errno_t sss_nss_mc_get_ctx(const char *name, struct sss_cli_mc_ctx *ctx)
{
struct stat fdstat;
@@ -129,19 +124,9 @@ errno_t sss_nss_mc_get_ctx(const char *name, struct sss_cli_mc_ctx *ctx)
goto done;
}
- ctx->fd = open(file, SSS_MC_OPEN_FLAGS);
+ ctx->fd = sss_open_cloexec(file, O_RDONLY, &ret);
if (ctx->fd == -1) {
- ret = EIO;
goto done;
-#ifndef O_CLOEXEC
- } else {
- int v;
-
- v = fcntl(ctx->fd, F_GETFD, 0);
- /* we ignore an error, it's not fatal and there is nothing we
- * can do about it anyways */
- (void)fcntl(ctx->fd, F_SETFD, v | FD_CLOEXEC);
-#endif
}
ret = fstat(ctx->fd, &fdstat);
diff --git a/src/tools/files.c b/src/tools/files.c
index 572617296b78e0443fdacf12443d7a9b76ebdd86..10fd6fe4396b3826a3ff65b8d84d3e0a1baddf5b 100644
--- a/src/tools/files.c
+++ b/src/tools/files.c
@@ -75,68 +75,6 @@ struct copy_ctx {
gid_t gid;
};
-static int open_cloexec(const char *pathname, int flags, int *ret)
-{
- int fd;
- int oflags;
-
- oflags = flags;
-#ifdef O_CLOEXEC
- oflags |= O_CLOEXEC;
-#endif
-
- errno = 0;
- fd = open(pathname, oflags);
- if (fd == -1) {
- if (ret) {
- *ret = errno;
- }
- return -1;
- }
-
-#ifndef O_CLOEXEC
- int v;
-
- v = fcntl(fd, F_GETFD, 0);
- /* we ignore an error, it's not fatal and there is nothing we
- * can do about it anyways */
- (void)fcntl(fd, F_SETFD, v | FD_CLOEXEC);
-#endif
-
- return fd;
-}
-
-static int openat_cloexec(int dir_fd, const char *pathname, int flags, int *ret)
-{
- int fd;
- int oflags;
-
- oflags = flags;
-#ifdef O_CLOEXEC
- oflags |= O_CLOEXEC;
-#endif
-
- errno = 0;
- fd = openat(dir_fd, pathname, oflags);
- if (fd == -1) {
- if (ret) {
- *ret = errno;
- }
- return -1;
- }
-
-#ifndef O_CLOEXEC
- int v;
-
- v = fcntl(fd, F_GETFD, 0);
- /* we ignore an error, it's not fatal and there is nothing we
- * can do about it anyways */
- (void)fcntl(fd, F_SETFD, v | FD_CLOEXEC);
-#endif
-
- return fd;
-}
-
static int sss_timeat_set(int dir_fd, const char *path,
const struct stat *statp,
int flags)
@@ -232,7 +170,7 @@ static int remove_tree_with_ctx(TALLOC_CTX *mem_ctx,
int ret, err;
int dir_fd;
- dir_fd = openat_cloexec(parent_fd, dir_name,
+ dir_fd = sss_openat_cloexec(parent_fd, dir_name,
O_RDONLY | O_DIRECTORY | O_NOFOLLOW, &ret);
if (dir_fd == -1) {
ret = errno;
@@ -607,7 +545,7 @@ copy_entry(struct copy_ctx *cctx,
* us against FIFOs and perhaps side-effects of the open() of a
* device file if there ever was one here, and doesn't matter
* for regular files or directories. */
- ifd = openat_cloexec(src_dir_fd, ent_name,
+ ifd = sss_openat_cloexec(src_dir_fd, ent_name,
O_RDONLY | O_NOFOLLOW | O_NONBLOCK, &ret);
if (ifd == -1 && ret != ELOOP) {
/* openat error */
@@ -721,7 +659,7 @@ copy_dir(struct copy_ctx *cctx,
goto done;
}
- dest_dir_fd = openat_cloexec(dest_parent_fd, dest_dir_name,
+ dest_dir_fd = sss_openat_cloexec(dest_parent_fd, dest_dir_name,
O_RDONLY | O_DIRECTORY | O_NOFOLLOW, &ret);
if (dest_dir_fd == -1) {
ret = errno;
@@ -807,7 +745,7 @@ int copy_tree(const char *src_root, const char *dst_root,
int fd = -1;
struct stat s_src;
- fd = open_cloexec(src_root, O_RDONLY | O_DIRECTORY, &ret);
+ fd = sss_open_cloexec(src_root, O_RDONLY | O_DIRECTORY, &ret);
if (fd == -1) {
goto fail;
}
diff --git a/src/util/io.c b/src/util/io.c
new file mode 100644
index 0000000000000000000000000000000000000000..e07a18c56b97b41409fed35066a7d6ab70cd753d
--- /dev/null
+++ b/src/util/io.c
@@ -0,0 +1,90 @@
+/*
+ SSSD
+
+ compact.c
+
+ Authors:
+ Lukas Slebodnik <lslebodn at redhat.com>
+
+ Copyright (C) 2012 Red Hat
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <unistd.h>
+#include <fcntl.h>
+
+#include "util/util.h"
+
+int sss_open_cloexec(const char *pathname, int flags, int *ret)
+{
+ int fd;
+ int oflags;
+
+ oflags = flags;
+#ifdef O_CLOEXEC
+ oflags |= O_CLOEXEC;
+#endif
+
+ errno = 0;
+ fd = open(pathname, oflags);
+ if (fd == -1) {
+ if (ret) {
+ *ret = errno;
+ }
+ return -1;
+ }
+
+#ifndef O_CLOEXEC
+ int v;
+
+ v = fcntl(fd, F_GETFD, 0);
+ /* we ignore an error, it's not fatal and there is nothing we
+ * can do about it anyways */
+ (void)fcntl(fd, F_SETFD, v | FD_CLOEXEC);
+#endif
+
+ return fd;
+}
+
+int sss_openat_cloexec(int dir_fd, const char *pathname, int flags, int *ret)
+{
+ int fd;
+ int oflags;
+
+ oflags = flags;
+#ifdef O_CLOEXEC
+ oflags |= O_CLOEXEC;
+#endif
+
+ errno = 0;
+ fd = openat(dir_fd, pathname, oflags);
+ if (fd == -1) {
+ if (ret) {
+ *ret = errno;
+ }
+ return -1;
+ }
+
+#ifndef O_CLOEXEC
+ int v;
+
+ v = fcntl(fd, F_GETFD, 0);
+ /* we ignore an error, it's not fatal and there is nothing we
+ * can do about it anyways */
+ (void)fcntl(fd, F_SETFD, v | FD_CLOEXEC);
+#endif
+
+ return fd;
+}
diff --git a/src/util/util.h b/src/util/util.h
index 8afb3bc964dfc7ea5debf8127b7c6bac08d11f42..d5ff9bb07efb48fee2c4f5bc6188e620ab4b87f1 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -584,6 +584,9 @@ errno_t sssd_domain_init(TALLOC_CTX *mem_ctx,
/* from util_lock.c */
errno_t sss_br_lock_file(int fd, size_t start, size_t len,
int num_tries, useconds_t wait);
+/* from io.c */
+int sss_open_cloexec(const char *pathname, int flags, int *ret);
+int sss_openat_cloexec(int dir_fd, const char *pathname, int flags, int *ret);
/* Endianness-compatibility for systems running older versions of glibc */
--
1.8.1.4
-------------- next part --------------
>From e1a78b0e030b08795703e4ac9efcea587b992e33 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Fri, 8 Mar 2013 15:27:14 +0100
Subject: [PATCH 2/2] More generalized function open_debug_file_ex()
Function open_debug_file_ex() set flag FD_CLOEXEC to opened
file according to the value of third parameter.
Removed duplicity of unsetting FD_CLOEXEC after calling function
open_debug_file_ex()
---
src/providers/krb5/krb5_init_shared.c | 6 +-----
src/providers/ldap/sdap_child_helpers.c | 6 +-----
src/util/debug.c | 10 ++++++----
src/util/util.h | 2 +-
4 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/src/providers/krb5/krb5_init_shared.c b/src/providers/krb5/krb5_init_shared.c
index 312c695ea1c8b8e03701db4185ba6f168d80cc8b..0dac5fa69af36e06189ce62392471700d4939572 100644
--- a/src/providers/krb5/krb5_init_shared.c
+++ b/src/providers/krb5/krb5_init_shared.c
@@ -30,7 +30,6 @@ errno_t krb5_child_init(struct krb5_ctx *krb5_auth_ctx,
{
errno_t ret;
FILE *debug_filep;
- unsigned v;
time_t renew_intv;
if (dp_opt_get_bool(krb5_auth_ctx->opts, KRB5_STORE_PASSWORD_IF_OFFLINE)) {
@@ -71,7 +70,7 @@ errno_t krb5_child_init(struct krb5_ctx *krb5_auth_ctx,
}
if (debug_to_file != 0) {
- ret = open_debug_file_ex(KRB5_CHILD_LOG_FILE, &debug_filep);
+ ret = open_debug_file_ex(KRB5_CHILD_LOG_FILE, &debug_filep, false);
if (ret != EOK) {
DEBUG(0, ("Error setting up logging (%d) [%s]\n",
ret, strerror(ret)));
@@ -84,9 +83,6 @@ errno_t krb5_child_init(struct krb5_ctx *krb5_auth_ctx,
ret = errno;
goto done;
}
-
- v = fcntl(krb5_auth_ctx->child_debug_fd, F_GETFD, 0);
- fcntl(krb5_auth_ctx->child_debug_fd, F_SETFD, v & ~FD_CLOEXEC);
}
done:
diff --git a/src/providers/ldap/sdap_child_helpers.c b/src/providers/ldap/sdap_child_helpers.c
index f2412f9e59a8ee3c6d1651b6b96ce2fd36c4fdd1..c63c3cb725c596f3203c017ab0d9891ed8d3248e 100644
--- a/src/providers/ldap/sdap_child_helpers.c
+++ b/src/providers/ldap/sdap_child_helpers.c
@@ -453,11 +453,10 @@ static errno_t set_tgt_child_timeout(struct tevent_req *req,
int setup_child(struct sdap_id_ctx *ctx)
{
int ret;
- unsigned v;
FILE *debug_filep;
if (debug_to_file != 0 && ldap_child_debug_fd == -1) {
- ret = open_debug_file_ex(LDAP_CHILD_LOG_FILE, &debug_filep);
+ ret = open_debug_file_ex(LDAP_CHILD_LOG_FILE, &debug_filep, false);
if (ret != EOK) {
DEBUG(0, ("Error setting up logging (%d) [%s]\n",
ret, strerror(ret)));
@@ -470,9 +469,6 @@ int setup_child(struct sdap_id_ctx *ctx)
ret = errno;
return ret;
}
-
- v = fcntl(ldap_child_debug_fd, F_GETFD, 0);
- fcntl(ldap_child_debug_fd, F_SETFD, v & ~FD_CLOEXEC);
}
return EOK;
diff --git a/src/util/debug.c b/src/util/debug.c
index 54fb8011be7a4c787b201314f1f08d1b46d3b66c..3bf5e75af0d7024ee7322f0c3574669a3e8bb4c0 100644
--- a/src/util/debug.c
+++ b/src/util/debug.c
@@ -173,7 +173,7 @@ void ldb_debug_messages(void *context, enum ldb_debug_level level,
free(message);
}
-int open_debug_file_ex(const char *filename, FILE **filep)
+int open_debug_file_ex(const char *filename, FILE **filep, bool want_cloexec)
{
FILE *f = NULL;
char *logpath;
@@ -214,8 +214,10 @@ int open_debug_file_ex(const char *filename, FILE **filep)
return EIO;
}
- flags = fcntl(debug_fd, F_GETFD, 0);
- (void) fcntl(debug_fd, F_SETFD, flags | FD_CLOEXEC);
+ if(want_cloexec) {
+ flags = fcntl(debug_fd, F_GETFD, 0);
+ (void) fcntl(debug_fd, F_SETFD, flags | FD_CLOEXEC);
+ }
if (filep == NULL) {
debug_file = f;
@@ -228,7 +230,7 @@ int open_debug_file_ex(const char *filename, FILE **filep)
int open_debug_file(void)
{
- return open_debug_file_ex(NULL, NULL);
+ return open_debug_file_ex(NULL, NULL, true);
}
int rotate_debug_files(void)
diff --git a/src/util/util.h b/src/util/util.h
index d5ff9bb07efb48fee2c4f5bc6188e620ab4b87f1..e61a94f7f5548ae2081a579307a38bf8ddcb4daf 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -336,7 +336,7 @@ safealign_memcpy(void *dest, const void *src, size_t n, size_t *counter)
/* From debug.c */
void ldb_debug_messages(void *context, enum ldb_debug_level level,
const char *fmt, va_list ap);
-int open_debug_file_ex(const char *filename, FILE **filep);
+int open_debug_file_ex(const char *filename, FILE **filep, bool want_cloexec);
int open_debug_file(void);
int rotate_debug_files(void);
void talloc_log_fn(const char *msg);
--
1.8.1.4
More information about the sssd-devel
mailing list