This is an automated email from the git hooks/post-receive script.
simo pushed a change to branch master in repository gssproxy.
from fca9a5a Fix NULL deref in gssi_release_cred() new eeaa36e Fix type of argument to gppint_get_def_creds() new 0c04bee Appease clang with memset instead of initializer
The 2 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "adds" were already present in the repository and have only been added to this reference.
Summary of changes: proxy/src/mechglue/gpp_creds.c | 4 +++- proxy/src/mechglue/gpp_init_sec_context.c | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-)
This is an automated email from the git hooks/post-receive script.
simo pushed a commit to branch master in repository gssproxy.
commit eeaa36e4ea3a0284ce970545e1d808c0a26e10f5 Author: Robbie Harwood rharwood@redhat.com Date: Mon Sep 19 21:41:08 2016 +0000
Fix type of argument to gppint_get_def_creds()
Some compilers don't like GSS_C_NO_NAME as a generic NULL value.
Signed-off-by: Robbie Harwood rharwood@redhat.com Reviewed-by: Simo Sorce simo@redhat.com --- proxy/src/mechglue/gpp_init_sec_context.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/proxy/src/mechglue/gpp_init_sec_context.c b/proxy/src/mechglue/gpp_init_sec_context.c index 2327b58..7b10400 100644 --- a/proxy/src/mechglue/gpp_init_sec_context.c +++ b/proxy/src/mechglue/gpp_init_sec_context.c @@ -156,7 +156,7 @@ OM_uint32 gssi_init_sec_context(OM_uint32 *minor_status,
maj = gppint_get_def_creds(&min, GPP_REMOTE_ONLY, - GSS_C_NO_NAME, + NULL, GSS_C_INITIATE, &r_creds); if (maj == GSS_S_COMPLETE) {
This is an automated email from the git hooks/post-receive script.
simo pushed a commit to branch master in repository gssproxy.
commit 0c04bee1c220812d5f55095688fb2d4256a71d43 Author: Robbie Harwood rharwood@redhat.com Date: Mon Sep 19 22:01:07 2016 +0000
Appease clang with memset instead of initializer
Signed-off-by: Robbie Harwood rharwood@redhat.com Reviewed-by: Simo Sorce simo@redhat.com Closes 31 --- proxy/src/mechglue/gpp_creds.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/proxy/src/mechglue/gpp_creds.c b/proxy/src/mechglue/gpp_creds.c index 2818dd7..31ad9d4 100644 --- a/proxy/src/mechglue/gpp_creds.c +++ b/proxy/src/mechglue/gpp_creds.c @@ -183,9 +183,11 @@ OM_uint32 gppint_get_def_creds(OM_uint32 *minor_status,
/* Then try with remote */ if (behavior == GPP_REMOTE_ONLY || behavior == GPP_REMOTE_FIRST) { - gssx_cred remote = {0}; + gssx_cred remote; gssx_cred *premote = NULL;
+ memset(&remote, 0, sizeof(gssx_cred)); + /* We intentionally ignore failures as finding creds is optional */ maj = retrieve_remote_creds(&min, name ? name->remote : NULL, &remote); if (maj == GSS_S_COMPLETE) {
This patch doesn't seem right to me.
Why doesn't "gssx_cred remote = {};" work?
On Tue, 2016-09-27 at 12:50 +0000, git repository hosting wrote:
This is an automated email from the git hooks/post-receive script.
simo pushed a commit to branch master in repository gssproxy.
commit 0c04bee1c220812d5f55095688fb2d4256a71d43 Author: Robbie Harwood rharwood@redhat.com Date: Mon Sep 19 22:01:07 2016 +0000
Appease clang with memset instead of initializer Signed-off-by: Robbie Harwood rharwood@redhat.com Reviewed-by: Simo Sorce simo@redhat.com Closes 31
proxy/src/mechglue/gpp_creds.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/proxy/src/mechglue/gpp_creds.c b/proxy/src/mechglue/gpp_creds.c index 2818dd7..31ad9d4 100644 --- a/proxy/src/mechglue/gpp_creds.c +++ b/proxy/src/mechglue/gpp_creds.c @@ -183,9 +183,11 @@ OM_uint32 gppint_get_def_creds(OM_uint32 *minor_status, /* Then try with remote */ if (behavior == GPP_REMOTE_ONLY || behavior == GPP_REMOTE_FIRST) { - gssx_cred remote = {0}; + gssx_cred remote; gssx_cred *premote = NULL; + memset(&remote, 0, sizeof(gssx_cred));
/* We intentionally ignore failures as finding creds is optional */ maj = retrieve_remote_creds(&min, name ? name->remote : NULL, &remote); if (maj == GSS_S_COMPLETE) {
On Tue, 2016-09-27 at 09:13 -0400, Nathaniel McCallum wrote:
This patch doesn't seem right to me.
Why doesn't "gssx_cred remote = {};" work?
I know it should work, and do not know why it didn't, but I pushed this patch anyway, I can put in something better later as long as clang does not complain.
Simo.
On Tue, 2016-09-27 at 12:50 +0000, git repository hosting wrote:
This is an automated email from the git hooks/post-receive script.
simo pushed a commit to branch master in repository gssproxy.
commit 0c04bee1c220812d5f55095688fb2d4256a71d43 Author: Robbie Harwood rharwood@redhat.com Date: Mon Sep 19 22:01:07 2016 +0000
Appease clang with memset instead of initializer Signed-off-by: Robbie Harwood <rharwood@redhat.com> Reviewed-by: Simo Sorce <simo@redhat.com> Closes 31
proxy/src/mechglue/gpp_creds.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/proxy/src/mechglue/gpp_creds.c b/proxy/src/mechglue/gpp_creds.c index 2818dd7..31ad9d4 100644 --- a/proxy/src/mechglue/gpp_creds.c +++ b/proxy/src/mechglue/gpp_creds.c @@ -183,9 +183,11 @@ OM_uint32 gppint_get_def_creds(OM_uint32 *minor_status,
/* Then try with remote */ if (behavior == GPP_REMOTE_ONLY || behavior == GPP_REMOTE_FIRST)
{
gssx_cred remote = {0};
gssx_cred remote; gssx_cred *premote = NULL;
memset(&remote, 0, sizeof(gssx_cred));
/* We intentionally ignore failures as finding creds is
optional */ maj = retrieve_remote_creds(&min, name ? name->remote : NULL, &remote); if (maj == GSS_S_COMPLETE) {
gss-proxy mailing list -- gss-proxy@lists.fedorahosted.org To unsubscribe send an email to gss-proxy-leave@lists.fedorahosted.org
On Tue, 2016-09-27 at 11:08 -0400, Simo Sorce wrote:
On Tue, 2016-09-27 at 09:13 -0400, Nathaniel McCallum wrote:
This patch doesn't seem right to me.
Why doesn't "gssx_cred remote = {};" work?
I know it should work, and do not know why it didn't, but I pushed this patch anyway, I can put in something better later as long as clang does not complain.
I suspect that clang is complaining because zero isn't the right value for the first field of gssx_cred. Simply removing that one byte should be sufficient to make clang happy.
What is the actual complaint?
Nathaniel McCallum npmccallum@redhat.com writes:
On Tue, 2016-09-27 at 11:08 -0400, Simo Sorce wrote:
On Tue, 2016-09-27 at 09:13 -0400, Nathaniel McCallum wrote:
Why doesn't "gssx_cred remote = {};" work?
I know it should work, and do not know why it didn't, but I pushed this patch anyway, I can put in something better later as long as clang does not complain.
I suspect that clang is complaining because zero isn't the right value for the first field of gssx_cred. Simply removing that one byte should be sufficient to make clang happy.
What is the actual complaint?
Clang says this:
src/mechglue/gpp_creds.c:186:29: warning: suggest braces around initialization of subobject [-Wmissing-braces] gssx_cred remote = {0}; ^ {} src/mechglue/gpp_creds.c:186:29: warning: suggest braces around initialization of subobject [-Wmissing-braces] gssx_cred remote = {0}; ^ {} 2 warnings generated.
and it's warning twice because what it wants to see is `gssx_cred remote = {{{0}}};`. Given the choice, I prefer the memset() to this notation.
On Tue, 2016-09-27 at 13:26 -0400, Robbie Harwood wrote:
Nathaniel McCallum npmccallum@redhat.com writes:
On Tue, 2016-09-27 at 11:08 -0400, Simo Sorce wrote:
On Tue, 2016-09-27 at 09:13 -0400, Nathaniel McCallum wrote:
Why doesn't "gssx_cred remote = {};" work?
I know it should work, and do not know why it didn't, but I pushed this patch anyway, I can put in something better later as long as clang does not complain.
I suspect that clang is complaining because zero isn't the right value for the first field of gssx_cred. Simply removing that one byte should be sufficient to make clang happy.
What is the actual complaint?
Clang says this:
src/mechglue/gpp_creds.c:186:29: warning: suggest braces around initialization of subobject [-Wmissing-braces] gssx_cred remote = {0}; ^ {} src/mechglue/gpp_creds.c:186:29: warning: suggest braces around initialization of subobject [-Wmissing-braces] gssx_cred remote = {0}; ^ {} 2 warnings generated.
and it's warning twice because what it wants to see is `gssx_cred remote = {{{0}}};`. Given the choice, I prefer the memset() to this notation.
Did you try taking the zero out?
I've seen this idiom before in our group and it (almost) never makes sense. {0} attempts to initialize the first scalar field to zero. It is functionally identical to {} when the first field of the struct is a scalar. But it often isn't. When the first field is, say, a struct, {0} is a syntax error.
C99 defines that when using an initializer, any unspecified fields are initialized to zero. Thus, {} is perfectly valid for all cases when initializing a struct to zero.
Using an empty initializer should be preferred because the compiler can simply align all the zeroed stack variables and do a single zero for all of them. Using memset() is harder to optimize this way.
Nathaniel
Nathaniel McCallum npmccallum@redhat.com writes:
On Tue, 2016-09-27 at 13:26 -0400, Robbie Harwood wrote:
Nathaniel McCallum npmccallum@redhat.com writes:
On Tue, 2016-09-27 at 11:08 -0400, Simo Sorce wrote:
On Tue, 2016-09-27 at 09:13 -0400, Nathaniel McCallum wrote:
Why doesn't "gssx_cred remote = {};" work?
I know it should work, and do not know why it didn't, but I pushed this patch anyway, I can put in something better later as long as clang does not complain.
I suspect that clang is complaining because zero isn't the right value for the first field of gssx_cred. Simply removing that one byte should be sufficient to make clang happy.
What is the actual complaint?
Clang says this:
src/mechglue/gpp_creds.c:186:29: warning: suggest braces around initialization of subobject [-Wmissing-braces] gssx_cred remote = {0}; ^ {} src/mechglue/gpp_creds.c:186:29: warning: suggest braces around initialization of subobject [-Wmissing-braces] gssx_cred remote = {0}; ^ {} 2 warnings generated.
and it's warning twice because what it wants to see is `gssx_cred remote = {{{0}}};`. Given the choice, I prefer the memset() to this notation.
Did you try taking the zero out?
I've seen this idiom before in our group and it (almost) never makes sense. {0} attempts to initialize the first scalar field to zero. It is functionally identical to {} when the first field of the struct is a scalar. But it often isn't. When the first field is, say, a struct, {0} is a syntax error.
C99 defines that when using an initializer, any unspecified fields are initialized to zero. Thus, {} is perfectly valid for all cases when initializing a struct to zero.
Wasn't aware of that. Good to know, thanks.
Using an empty initializer should be preferred because the compiler can simply align all the zeroed stack variables and do a single zero for all of them. Using memset() is harder to optimize this way.
On my machine (x86_64 clang-3.6.2), the two compile to the same code, and that code is what memset() (i.e., bzero()) also compiles to.
It doesn't really matter to me which we use; I went with the version I immediately understood.
On Tue, 2016-09-27 at 15:05 -0400, Robbie Harwood wrote:
Nathaniel McCallum npmccallum@redhat.com writes:
On Tue, 2016-09-27 at 13:26 -0400, Robbie Harwood wrote:
Nathaniel McCallum npmccallum@redhat.com writes:
On Tue, 2016-09-27 at 11:08 -0400, Simo Sorce wrote:
On Tue, 2016-09-27 at 09:13 -0400, Nathaniel McCallum wrote:
Why doesn't "gssx_cred remote = {};" work?
I know it should work, and do not know why it didn't, but I pushed this patch anyway, I can put in something better later as long as clang does not complain.
I suspect that clang is complaining because zero isn't the right value for the first field of gssx_cred. Simply removing that one byte should be sufficient to make clang happy.
What is the actual complaint?
Clang says this:
src/mechglue/gpp_creds.c:186:29: warning: suggest braces around initialization of subobject [-Wmissing-braces] gssx_cred remote = {0}; ^ {} src/mechglue/gpp_creds.c:186:29: warning: suggest braces around initialization of subobject [-Wmissing-braces] gssx_cred remote = {0}; ^ {} 2 warnings generated.
and it's warning twice because what it wants to see is `gssx_cred remote = {{{0}}};`. Given the choice, I prefer the memset() to this notation.
Did you try taking the zero out?
I've seen this idiom before in our group and it (almost) never makes sense. {0} attempts to initialize the first scalar field to zero. It is functionally identical to {} when the first field of the struct is a scalar. But it often isn't. When the first field is, say, a struct, {0} is a syntax error.
C99 defines that when using an initializer, any unspecified fields are initialized to zero. Thus, {} is perfectly valid for all cases when initializing a struct to zero.
Wasn't aware of that. Good to know, thanks.
Using an empty initializer should be preferred because the compiler can simply align all the zeroed stack variables and do a single zero for all of them. Using memset() is harder to optimize this way.
On my machine (x86_64 clang-3.6.2), the two compile to the same code, and that code is what memset() (i.e., bzero()) also compiles to.
It doesn't really matter to me which we use; I went with the version I immediately understood.
It probably isn't worth changing now. I think it is just best to use the syntax I provided in the future since it is the most readable. There are some cases (like VLAs) where this syntax doesn't work, but I think it should be preferred.
Nathaniel
gss-proxy@lists.fedorahosted.org