Hi,
I investigated the situation around the log message, which mentioned Lukas. I prepared this patch. The result is that the original message
[sssd[be[cygnus.dev]]] [be_get_account_info] (0x0200): Got request
for [0x1001][1][name=celestian] changed to
[sssd[be[cygnus.dev]]] [be_get_account_info] (0x0200): Got request
for [0x1001][FAST BE_REQ_USER][1][name=celestian]
A) I would like to ask if mark 'FAST' is useful, or if I should remove it.
B) While writing a patch Lukas noticed another similar logging messages
[sssd[pam]] [sss_dp_get_account_msg] (0x0400): Creating request for
[LDAP][3][1][name=mof_user6]
I investigated it. This is the same thing -- BE_REQ_*, but it is no longer in the provider, but in the responder. Can you please advise me where I could the function 'be_req2str' write?
The first message is coming from
src/providers/data_provider_be.c --> be_get_account_info,
the second is from
src/responder/common/responder_dp --> sss_dp_get_account_msg
Thanks.
Petr
On 07/15/2015 08:53 AM, Petr Cech wrote:
Hi,
I investigated the situation around the log message, which mentioned Lukas. I prepared this patch. The result is that the original message
[sssd[be[cygnus.dev]]] [be_get_account_info] (0x0200): Got request
for [0x1001][1][name=celestian] changed to
[sssd[be[cygnus.dev]]] [be_get_account_info] (0x0200): Got request
for [0x1001][FAST BE_REQ_USER][1][name=celestian]
A) I would like to ask if mark 'FAST' is useful, or if I should remove it.
I think it can't hurt and might be useful, so IMO leave it.
B) While writing a patch Lukas noticed another similar logging messages
[sssd[pam]] [sss_dp_get_account_msg] (0x0400): Creating request for
[LDAP][3][1][name=mof_user6]
I investigated it. This is the same thing -- BE_REQ_*, but it is no longer in the provider, but in the responder. Can you please advise me where I could the function 'be_req2str' write?
I think you should move it to separate file, as you don't want to share more code then necessary. There are 2 possibilities where to have this file a) in src/providers - responders already link with some modules from this folder, so I suppose it's viable
SSSD_RESPONDER_OBJ = \ ... src/providers/data_provider_iface_generated.c \ src/providers/data_provider_iface_generated.h
b) move it to src/util which is a natural place for shared code
In my opinion a) is better because code is closely coupled with back end and is not general enough to be part of utils.
Sorry for not giving definitive answers. :-)
+const char* be_req2str(dbus_uint32_t req_type) +{
// I was thinking whether you should use some static char output[??];
//and fill it with "FAST" in a single condition and thus simplifying subsequent case
if (req_type & BE_REQ_FAST0) sprintf(output, "FAST");
- switch (req_type & BE_REQ_TYPE_MASK) {
- case BE_REQ_USER:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_USER" : "BE_REQ_USER";- case BE_REQ_GROUP:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_GROUP" : "BE_REQ_GROUP";- case BE_REQ_INITGROUPS:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_INITGROUPS" : "BE_REQ_INITGROUPS";- case BE_REQ_NETGROUP:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_NETGROUP" : "BE_REQ_NETGROUP";- case BE_REQ_SERVICES:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_SERVICES" : "BE_REQ_SERVICES";- case BE_REQ_SUDO_FULL:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_SUDO_FULL" : "BE_REQ_SUDO_FULL";- case BE_REQ_SUDO_RULES:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_SUDO_RULES" : "BE_REQ_SUDO_RULES";- case BE_REQ_AUTOFS:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_AUTOFS" : "BE_REQ_AUTOFS";- case BE_REQ_HOST:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_HOST" : "BE_REQ_HOST";- case BE_REQ_BY_SECID:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_BY_SECID" : "BE_REQ_BY_SECID";- case BE_REQ_USER_AND_GROUP:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_USER_AND_GROUP" : "BE_REQ_USER_AND_GROUP";- case BE_REQ_BY_UUID:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_BY_UUID" : "BE_REQ_BY_UUID";- case BE_REQ_BY_CERT:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_BY_CERT" : "BE_REQ_BY_CERT";- }
- return "UNKNOWN_REQ";
+}
The first message is coming from
src/providers/data_provider_be.c --> be_get_account_info,
the second is from
src/responder/common/responder_dp --> sss_dp_get_account_msg
Thanks.
Petr
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On Wed, Aug 05, 2015 at 10:03:30AM +0200, Pavel Reichl wrote:
On 07/15/2015 08:53 AM, Petr Cech wrote:
Hi,
I investigated the situation around the log message, which mentioned Lukas. I prepared this patch. The result is that the original message
[sssd[be[cygnus.dev]]] [be_get_account_info] (0x0200): Got request for
[0x1001][1][name=celestian] changed to
[sssd[be[cygnus.dev]]] [be_get_account_info] (0x0200): Got request for
[0x1001][FAST BE_REQ_USER][1][name=celestian]
A) I would like to ask if mark 'FAST' is useful, or if I should remove it.
I think it can't hurt and might be useful, so IMO leave it.
Yes, IIRC FAST flag is always set for identity lookups and never set for PAM responder lookups. I think it tells the responder whether to attempt to reconnect if offline.
This might be useful data for the Internal document.
B) While writing a patch Lukas noticed another similar logging messages
[sssd[pam]] [sss_dp_get_account_msg] (0x0400): Creating request for
[LDAP][3][1][name=mof_user6]
I investigated it. This is the same thing -- BE_REQ_*, but it is no longer in the provider, but in the responder. Can you please advise me where I could the function 'be_req2str' write?
I think you should move it to separate file, as you don't want to share more code then necessary. There are 2 possibilities where to have this file a) in src/providers - responders already link with some modules from this folder, so I suppose it's viable
Yes, this is a bit of hierarchy violation, but in the end we need to share the constants somehow. I think it's fine to keep the definite version in the providers/dp_* hierarchy, because that's where the interface is defined, the responder is a consumer.
SSSD_RESPONDER_OBJ = \ ... src/providers/data_provider_iface_generated.c \ src/providers/data_provider_iface_generated.h
b) move it to src/util which is a natural place for shared code
In my opinion a) is better because code is closely coupled with back end and is not general enough to be part of utils.
I agree. In future, especially if we document the interface and promote 3rd parties to write responders and providers, we might want to put the interface into its own directory, but for now it's fine.
Sorry for not giving definitive answers. :-)
+const char* be_req2str(dbus_uint32_t req_type) +{
// I was thinking whether you should use some static char output[??];
//and fill it with "FAST" in a single condition and thus simplifying subsequent case
if (req_type & BE_REQ_FAST0) sprintf(output, "FAST");
I think this would be fine if we formatted the output, but we only return const strings..
- switch (req_type & BE_REQ_TYPE_MASK) {
- case BE_REQ_USER:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_USER" : "BE_REQ_USER";- case BE_REQ_GROUP:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_GROUP" : "BE_REQ_GROUP";- case BE_REQ_INITGROUPS:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_INITGROUPS" : "BE_REQ_INITGROUPS";- case BE_REQ_NETGROUP:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_NETGROUP" : "BE_REQ_NETGROUP";- case BE_REQ_SERVICES:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_SERVICES" : "BE_REQ_SERVICES";- case BE_REQ_SUDO_FULL:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_SUDO_FULL" : "BE_REQ_SUDO_FULL";- case BE_REQ_SUDO_RULES:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_SUDO_RULES" : "BE_REQ_SUDO_RULES";- case BE_REQ_AUTOFS:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_AUTOFS" : "BE_REQ_AUTOFS";- case BE_REQ_HOST:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_HOST" : "BE_REQ_HOST";- case BE_REQ_BY_SECID:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_BY_SECID" : "BE_REQ_BY_SECID";- case BE_REQ_USER_AND_GROUP:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_USER_AND_GROUP" : "BE_REQ_USER_AND_GROUP";- case BE_REQ_BY_UUID:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_BY_UUID" : "BE_REQ_BY_UUID";- case BE_REQ_BY_CERT:
return req_type & BE_REQ_FAST ? "FAST BE_REQ_BY_CERT" : "BE_REQ_BY_CERT";- }
- return "UNKNOWN_REQ";
+}
The first message is coming from
src/providers/data_provider_be.c --> be_get_account_info,
the second is from
src/responder/common/responder_dp --> sss_dp_get_account_msg
Thanks.
Petr
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On 08/05/2015 11:23 AM, Jakub Hrozek wrote:
B)
While writing a patch Lukas noticed another similar logging messages
>[sssd[pam]] [sss_dp_get_account_msg] (0x0400): Creating request for
[LDAP][3][1][name=mof_user6]
I investigated it. This is the same thing -- BE_REQ_*, but it is no longer in the provider, but in the responder. Can you please advise me where I could the function 'be_req2str' write?
I think you should move it to separate file, as you don't want to share more code then necessary. There are 2 possibilities where to have this file a) in src/providers - responders already link with some modules from this folder, so I suppose it's viable
Yes, this is a bit of hierarchy violation, but in the end we need to share the constants somehow. I think it's fine to keep the definite version in the providers/dp_* hierarchy, because that's where the interface is defined, the responder is a consumer.
There is new patch attached.
I think, that constants and const2str() functions should be in one place. I tried to suggest how we might share our constants. That's why I created a new header file in which we could move all the constants in the future.
I am open to discussion. I look forward to your views.
Petr
On 08/18/2015 01:19 PM, Petr Cech wrote:
On 08/05/2015 11:23 AM, Jakub Hrozek wrote:
B)
While writing a patch Lukas noticed another similar logging
messages
>>[sssd[pam]] [sss_dp_get_account_msg] (0x0400): Creating
request for
[LDAP][3][1][name=mof_user6]
I investigated it. This is the same thing -- BE_REQ_*, but it is
no longer
in the provider, but in the responder. Can you please advise me
where I
could the function 'be_req2str' write?
I think you should move it to separate file, as you don't want to
share more
code then necessary. There are 2 possibilities where to have this file a) in src/providers - responders already link with some modules
from this
folder, so I suppose it's viable
Yes, this is a bit of hierarchy violation, but in the end we need to share the constants somehow. I think it's fine to keep the definite version in the providers/dp_* hierarchy, because that's where the interface is defined, the responder is a consumer.
There is new patch attached.
I think, that constants and const2str() functions should be in one place. I tried to suggest how we might share our constants. That's why I created a new header file in which we could move all the constants in the future.
I am open to discussion. I look forward to your views.
Petr
Hello Petr,
make dict check fails:
/workspace/ci/label/rhel7/ci-build-debug/sssd-1.13.1/_inst/share/locale" -g3 -O2 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -c ../src/providers/ldap/ldap_id.c -fPIC -DPIC -o src/providers/ldap/.libs/libsss_ldap_common_la-ldap_id.o ../src/providers/ldap/ldap_id.c:33:41: fatal error: providers/data_provider_req.h: No such file or directory #include "providers/data_provider_req.h"
you can fix this by something like:
Makefile.am @@ -584,6 +584,7 @@ dist_noinst_HEADERS = \ src/confdb/confdb_private.h \ src/confdb/confdb_setup.h \ src/providers/data_provider.h \
- src/providers/data_provider_req.h \
I think that data_provider_req.h should be included in data_provider.h, you could save a lot of changes in source files that require both of them.
On 08/21/2015 01:08 PM, Pavel Reichl wrote:
Hello Petr,
make dict check fails:
/workspace/ci/label/rhel7/ci-build-debug/sssd-1.13.1/_inst/share/locale" -g3 -O2 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -c ../src/providers/ldap/ldap_id.c -fPIC -DPIC -o src/providers/ldap/.libs/libsss_ldap_common_la-ldap_id.o ../src/providers/ldap/ldap_id.c:33:41: fatal error: providers/data_provider_req.h: No such file or directory #include "providers/data_provider_req.h"
you can fix this by something like:
Makefile.am @@ -584,6 +584,7 @@ dist_noinst_HEADERS = \ src/confdb/confdb_private.h \ src/confdb/confdb_setup.h \ src/providers/data_provider.h \
- src/providers/data_provider_req.h \
I think that data_provider_req.h should be included in data_provider.h, you could save a lot of changes in source files that require both of them.
Thanks. There is repaired patch attached. Petr
On 08/21/2015 03:40 PM, Petr Cech wrote:
On 08/21/2015 01:08 PM, Pavel Reichl wrote:
Hello Petr,
make dict check fails:
/workspace/ci/label/rhel7/ci-build-debug/sssd-1.13.1/_inst/share/locale" -g3 -O2 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -c ../src/providers/ldap/ldap_id.c -fPIC -DPIC -o src/providers/ldap/.libs/libsss_ldap_common_la-ldap_id.o ../src/providers/ldap/ldap_id.c:33:41: fatal error: providers/data_provider_req.h: No such file or directory #include "providers/data_provider_req.h"
you can fix this by something like:
Makefile.am @@ -584,6 +584,7 @@ dist_noinst_HEADERS = \ src/confdb/confdb_private.h \ src/confdb/confdb_setup.h \ src/providers/data_provider.h \
- src/providers/data_provider_req.h \
I think that data_provider_req.h should be included in data_provider.h, you could save a lot of changes in source files that require both of them.
Thanks. There is repaired patch attached. Petr
Petr can you change data_provider_req.c to include
"providers/data_provider_req.h"
instead of
"providers/data_provider.h" ?
I originally thought that you will be able to include solely data_provider_req.h from responder_dp.c but I see that data_provider.h is required. But I still don't mind introducing data_provider_req.h.
If you change this little nitpick I think I can ACK the patch.
On 08/21/2015 05:10 PM, Pavel Reichl wrote:
Petr can you change data_provider_req.c to include
"providers/data_provider_req.h"
instead of
"providers/data_provider.h" ?
I originally thought that you will be able to include solely data_provider_req.h from responder_dp.c but I see that data_provider.h is required. But I still don't mind introducing data_provider_req.h.
If you change this little nitpick I think I can ACK the patch.
Pavel, you're right, that's mine main opinion to this issue. I am sorry, I need more focus and coffe. There is fixed (not repaired) patch. Petr
On 08/21/2015 05:49 PM, Petr Cech wrote:
On 08/21/2015 05:10 PM, Pavel Reichl wrote:
Petr can you change data_provider_req.c to include
"providers/data_provider_req.h"
instead of
"providers/data_provider.h" ?
I originally thought that you will be able to include solely data_provider_req.h from responder_dp.c but I see that data_provider.h is required. But I still don't mind introducing data_provider_req.h.
If you change this little nitpick I think I can ACK the patch.
Pavel, you're right, that's mine main opinion to this issue. I am sorry, I need more focus and coffe. There is fixed (not repaired) patch. Petr
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
ACK
[snip]
On 08/21/2015 05:10 PM, Pavel Reichl wrote:
ACK
Just adding link to passed ci (with exception of test test_ldap_id_cleanup which is false positive (and already fixed and acked on list)): http://sssd-ci.duckdns.org/logs/job/23/73/summary.html
On (21/08/15 17:49), Petr Cech wrote:
On 08/21/2015 05:10 PM, Pavel Reichl wrote:
Petr can you change data_provider_req.c to include
"providers/data_provider_req.h"
instead of
"providers/data_provider.h" ?
I originally thought that you will be able to include solely data_provider_req.h from responder_dp.c but I see that data_provider.h is required. But I still don't mind introducing data_provider_req.h.
If you change this little nitpick I think I can ACK the patch.
Pavel, you're right, that's mine main opinion to this issue. I am sorry, I need more focus and coffe. There is fixed (not repaired) patch. Petr
From aebda5def026d7a0fc40c4034ef18ba97ada5f36 Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Tue, 18 Aug 2015 06:59:31 -0400 Subject: [PATCH] DATA_PROVIDER: BE_REQ as string in log message
Add be_req2str() for translation BE_REQ to string. So we will have || Got request for [0x1001][FAST BE_REQ_USER][1][name=celestian] instead of || Got request for [0x1001][1][name=celestian]
Function be_req2str() is used in data provider and in responder too. So this patch create new header file data_provider_req.h which delivers function be_req2str() and definitions of BE_REQ_*.
Resolves: https://fedorahosted.org/sssd/ticket/2708
Makefile.am | 6 +++- src/providers/data_provider.h | 17 +--------- src/providers/data_provider_be.c | 3 +- src/providers/data_provider_req.c | 68 +++++++++++++++++++++++++++++++++++++ src/providers/data_provider_req.h | 51 ++++++++++++++++++++++++++++ src/responder/common/responder_dp.c | 4 +-- 6 files changed, 129 insertions(+), 20 deletions(-) create mode 100644 src/providers/data_provider_req.c create mode 100644 src/providers/data_provider_req.h
diff --git a/Makefile.am b/Makefile.am index f153ab0adf390880672a1681b386ea26426465cb..94920b29d7aab44085e401f8ada8555ab69fed6a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -446,7 +446,8 @@ SSSD_RESPONDER_OBJ = \ src/monitor/monitor_iface_generated.c \ src/monitor/monitor_iface_generated.h \ src/providers/data_provider_iface_generated.c \
- src/providers/data_provider_iface_generated.h
- src/providers/data_provider_iface_generated.h \
- src/providers/data_provider_req.c
SSSD_TOOLS_OBJ = \ src/tools/sss_sync_ops.c \ @@ -583,6 +584,7 @@ dist_noinst_HEADERS = \ src/confdb/confdb_private.h \ src/confdb/confdb_setup.h \ src/providers/data_provider.h \
- src/providers/data_provider_req.h \ src/providers/dp_backend.h \ src/providers/dp_dyndns.h \ src/providers/dp_ptask_private.h \
@@ -1193,6 +1195,7 @@ endif
sssd_be_SOURCES = \ src/providers/data_provider_be.c \
- src/providers/data_provider_req.c \ src/providers/data_provider_fo.c \ src/providers/data_provider_opts.c \ src/providers/data_provider_callbacks.c \
@@ -1646,6 +1649,7 @@ simple_access_tests_SOURCES = \ src/providers/simple/simple_access.c \ src/providers/simple/simple_access_check.c \ src/providers/data_provider_be.c \
- src/providers/data_provider_req.c \ src/providers/data_provider_fo.c \ src/providers/data_provider_opts.c \ src/providers/data_provider_callbacks.c \
diff --git a/src/providers/data_provider.h b/src/providers/data_provider.h index 510c63ce41c99314ec8fcf11fffb2e66082e8951..39051b90c3aad96f62dcbb86a20bcfd8c954879b 100644 --- a/src/providers/data_provider.h +++ b/src/providers/data_provider.h @@ -43,6 +43,7 @@ #include "sbus/sbus_client.h" #include "sss_client/sss_cli.h" #include "util/authtok.h" +#include "providers/data_provider_req.h" #include "providers/data_provider_iface_generated.h"
#define DATA_PROVIDER_VERSION 0x0001 @@ -131,22 +132,6 @@ #define BE_FILTER_CERT 6 #define BE_FILTER_WILDCARD 7
-#define BE_REQ_USER 0x0001 -#define BE_REQ_GROUP 0x0002 -#define BE_REQ_INITGROUPS 0x0003 -#define BE_REQ_NETGROUP 0x0004 -#define BE_REQ_SERVICES 0x0005 -#define BE_REQ_SUDO_FULL 0x0006 -#define BE_REQ_SUDO_RULES 0x0007 -#define BE_REQ_AUTOFS 0x0009 -#define BE_REQ_HOST 0x0010 -#define BE_REQ_BY_SECID 0x0011 -#define BE_REQ_USER_AND_GROUP 0x0012 -#define BE_REQ_BY_UUID 0x0013 -#define BE_REQ_BY_CERT 0x0014 -#define BE_REQ_TYPE_MASK 0x00FF -#define BE_REQ_FAST 0x1000
#define DP_SEC_ID "secid" #define DP_CERT "cert" /* sizeof() counts the trailing \0 so we must substract 1 for the string diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index d147630248f0a24f5a632760b55b9284a6928e40..d71a69cb8e2997975828236998ec0b0e3f353f07 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -1104,7 +1104,8 @@ static int be_get_account_info(struct sbus_request *dbus_req, void *user_data) return EOK; /* handled */
DEBUG(SSSDBG_FUNC_DATA,
"Got request for [%#x][%d][%s]\n", type, attr_type, filter);
"Got request for [%#x][%s][%d][%s]\n", type, be_req2str(type),attr_type, filter);/* If we are offline and fast reply was requested
- return offline immediately
diff --git a/src/providers/data_provider_req.c b/src/providers/data_provider_req.c new file mode 100644 index 0000000000000000000000000000000000000000..2a7e4fed2f19f75bd2504b915e6a8c75a1482614 --- /dev/null +++ b/src/providers/data_provider_req.c @@ -0,0 +1,68 @@ +/*
- SSSD
- Data Provider -- backend request
- Copyright (C) Petr Cech pcech@redhat.com 2015
- 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 "providers/data_provider_req.h"
+const char *be_req2str(dbus_uint32_t req_type) +{
- switch (req_type & BE_REQ_TYPE_MASK) {
- case BE_REQ_USER:
return req_type & BE_REQ_FAST ?"FAST BE_REQ_USER" : "BE_REQ_USER";
Petr, what do you think about small simplification.
#include "providers/data_provider_req.h"
+#define be_req_to_str(req_type, be_req_t) \ + ((req_type) & BE_REQ_FAST) \ + ? "FAST " #be_req_t \ + : #be_req_t + const char *be_req2str(dbus_uint32_t req_type) { switch (req_type & BE_REQ_TYPE_MASK) { case BE_REQ_USER: - return req_type & BE_REQ_FAST ? - "FAST BE_REQ_USER" : "BE_REQ_USER"; + return be_req_to_str(req_type, BE_REQ_USER); case BE_REQ_GROUP: return req_type & BE_REQ_FAST ? "FAST BE_REQ_GROUP" : "BE_REQ_GROUP";
The result of preprocessor is almost the same and code look little bit simpler.
const char *be_req2str(dbus_uint32_t req_type) { switch (req_type & 0x00FF) { case 0x0001: return ((req_type) & 0x1000) ? "FAST " "BE_REQ_USER" : "BE_REQ_USER"; case 0x0002: return req_type & 0x1000 ? "FAST BE_REQ_GROUP" : "BE_REQ_GROUP"; case 0x0003: return req_type & 0x1000 ? "FAST BE_REQ_INITGROUPS" : "BE_REQ_INITGROUPS";
Feel free to rename macro; it's just a POC version.
LS
On 08/27/2015 01:26 PM, Lukas Slebodnik wrote:
Petr, what do you think about small simplification.
#include "providers/data_provider_req.h"
+#define be_req_to_str(req_type, be_req_t) \
((req_type) & BE_REQ_FAST) \? "FAST " #be_req_t \: #be_req_t- const char *be_req2str(dbus_uint32_t req_type) { switch (req_type & BE_REQ_TYPE_MASK) { case BE_REQ_USER:
return req_type & BE_REQ_FAST ?"FAST BE_REQ_USER" : "BE_REQ_USER";
return be_req_to_str(req_type, BE_REQ_USER); case BE_REQ_GROUP: return req_type & BE_REQ_FAST ? "FAST BE_REQ_GROUP" : "BE_REQ_GROUP";The result of preprocessor is almost the same and code look little bit simpler.
const char *be_req2str(dbus_uint32_t req_type) { switch (req_type & 0x00FF) { case 0x0001: return ((req_type) & 0x1000) ? "FAST " "BE_REQ_USER" : "BE_REQ_USER"; case 0x0002: return req_type & 0x1000 ? "FAST BE_REQ_GROUP" : "BE_REQ_GROUP"; case 0x0003: return req_type & 0x1000 ? "FAST BE_REQ_INITGROUPS" : "BE_REQ_INITGROUPS";
Feel free to rename macro; it's just a POC version.
LS
Thanks.
There is improved patch attached.
Petr
On 09/04/2015 04:32 PM, Pavel Reichl wrote:
On 08/28/2015 04:31 PM, Petr Cech wrote:
"Got request for [%#x][%s][%d][%s]\n", type, be_req2str(type),attr_type, filter);Petr do you think it could be useful to print attr_type as a string?
We talked about it offline. It seems to be only value type, nothing important.
But there is a new rebased patch, because development you cannot stop.
Petr
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On 09/08/2015 04:28 PM, Petr Cech wrote:
On 09/04/2015 04:32 PM, Pavel Reichl wrote:
On 08/28/2015 04:31 PM, Petr Cech wrote:
"Got request for [%#x][%s][%d][%s]\n", type, be_req2str(type),attr_type, filter);Petr do you think it could be useful to print attr_type as a string?
We talked about it offline. It seems to be only value type, nothing important.
OK, thanks for looking into it,
But there is a new rebased patch, because development you cannot stop.
:-)
ci passed: http://sssd-ci.duckdns.org/logs/job/25/62/summary.html
ACK
On Tue, Sep 08, 2015 at 05:53:27PM +0200, Pavel Reichl wrote:
On 09/08/2015 04:28 PM, Petr Cech wrote:
On 09/04/2015 04:32 PM, Pavel Reichl wrote:
On 08/28/2015 04:31 PM, Petr Cech wrote:
"Got request for [%#x][%s][%d][%s]\n", type, be_req2str(type),attr_type, filter);Petr do you think it could be useful to print attr_type as a string?
We talked about it offline. It seems to be only value type, nothing important.
OK, thanks for looking into it,
But there is a new rebased patch, because development you cannot stop.
:-)
ci passed: http://sssd-ci.duckdns.org/logs/job/25/62/summary.html
ACK
Sorry for the delay in pushing. Pushed to master now as 376eaf187c13c2a1eaea0ffbdd970b6b563ab74c
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Lukas Slebodnik -
Pavel Reichl -
Petr Cech