nscd: Use errval, not errno to guide cache update (bug 30662)

Message ID 87zg3ps23z.fsf@oldenburg.str.redhat.com
State Committed
Commit 2d472b48610f6a298d28035b683ab13e9afac4cb
Headers
Series nscd: Use errval, not errno to guide cache update (bug 30662) |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm warning Patch failed to apply
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 warning Patch failed to apply

Commit Message

Florian Weimer July 21, 2023, 3:42 p.m. UTC
  The errno variable is potentially clobbered by the preceding
send call.  It is not related to the to-be-cached information.
The parallel code in hstcache.c and servicescache.c already uses
errval.

---
 nscd/grpcache.c | 2 +-
 nscd/pwdcache.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)


base-commit: 2c6b4b272e6b4d07303af25709051c3e96288f2d
  

Comments

Siddhesh Poyarekar July 21, 2023, 4:43 p.m. UTC | #1
On 2023-07-21 11:42, Florian Weimer via Libc-alpha wrote:
> The errno variable is potentially clobbered by the preceding
> send call.  It is not related to the to-be-cached information.
> The parallel code in hstcache.c and servicescache.c already uses
> errval.

Looks OK to me.  +Andreas for visibility but I reckon it's OK to push 
since it's a bug fix and the release branching is at least a week away.

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

> 
> ---
>   nscd/grpcache.c | 2 +-
>   nscd/pwdcache.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/nscd/grpcache.c b/nscd/grpcache.c
> index cdd1071970..2d01b84519 100644
> --- a/nscd/grpcache.c
> +++ b/nscd/grpcache.c
> @@ -116,7 +116,7 @@ cache_addgr (struct database_dyn *db, int fd, request_header *req,
>   
>   	  /* If we have a transient error or cannot permanently store
>   	     the result, so be it.  */
> -	  if (errno == EAGAIN || __builtin_expect (db->negtimeout == 0, 0))
> +	  if (errval == EAGAIN || __glibc_unlikely (db->negtimeout == 0))
>   	    {
>   	      /* Mark the old entry as obsolete.  */
>   	      if (dh != NULL)
> diff --git a/nscd/pwdcache.c b/nscd/pwdcache.c
> index e1b579de6b..e5d51e74ff 100644
> --- a/nscd/pwdcache.c
> +++ b/nscd/pwdcache.c
> @@ -122,7 +122,7 @@ cache_addpw (struct database_dyn *db, int fd, request_header *req,
>   
>   	  /* If we have a transient error or cannot permanently store
>   	     the result, so be it.  */
> -	  if (errno == EAGAIN || __builtin_expect (db->negtimeout == 0, 0))
> +	  if (errval == EAGAIN || __glibc_unlikely (db->negtimeout == 0))
>   	    {
>   	      /* Mark the old entry as obsolete.  */
>   	      if (dh != NULL)
> 
> base-commit: 2c6b4b272e6b4d07303af25709051c3e96288f2d
>
  
Andreas K. Huettel July 21, 2023, 5:38 p.m. UTC | #2
> 
> Looks OK to me.  +Andreas for visibility but I reckon it's OK to push 
> since it's a bug fix and the release branching is at least a week away.
> 
Exactly. Thanks!
  

Patch

diff --git a/nscd/grpcache.c b/nscd/grpcache.c
index cdd1071970..2d01b84519 100644
--- a/nscd/grpcache.c
+++ b/nscd/grpcache.c
@@ -116,7 +116,7 @@  cache_addgr (struct database_dyn *db, int fd, request_header *req,
 
 	  /* If we have a transient error or cannot permanently store
 	     the result, so be it.  */
-	  if (errno == EAGAIN || __builtin_expect (db->negtimeout == 0, 0))
+	  if (errval == EAGAIN || __glibc_unlikely (db->negtimeout == 0))
 	    {
 	      /* Mark the old entry as obsolete.  */
 	      if (dh != NULL)
diff --git a/nscd/pwdcache.c b/nscd/pwdcache.c
index e1b579de6b..e5d51e74ff 100644
--- a/nscd/pwdcache.c
+++ b/nscd/pwdcache.c
@@ -122,7 +122,7 @@  cache_addpw (struct database_dyn *db, int fd, request_header *req,
 
 	  /* If we have a transient error or cannot permanently store
 	     the result, so be it.  */
-	  if (errno == EAGAIN || __builtin_expect (db->negtimeout == 0, 0))
+	  if (errval == EAGAIN || __glibc_unlikely (db->negtimeout == 0))
 	    {
 	      /* Mark the old entry as obsolete.  */
 	      if (dh != NULL)