[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2] tools/9pfsd: Fix build error caused by strerror_r()


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Henry Wang <xin.wang2@xxxxxxx>
  • Date: Thu, 7 Mar 2024 20:19:40 +0800
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=suse.com smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0)
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=k0gcwpiaF4ajbC6fPVDyeiKq/4opE2bwQfKz6n50CTQ=; b=GRnLqLsltyQcv7FFQM4f2jasKXgG4JDcGuV8+FNkuOkvH0FTOoplhAWqYuyMI0LR4jhJOidiOyFhMoff1QzX1VOGW3GnnoxTfB8IuBplT1QCvsVML/j5rG+9N2shM9PxYYRk4FS3Z2K/5dpVFJ6eGKsmDjh9lw6ooI5mYZYMXALnSYe6XN46q7QUI5GrtxG9fiRCnXDcYHA1sPwJd9FgssQt6qR4MF37UM/5tfGhk0RGEx57Noza/uwyscquOjkIB00bfwUvp3oZVfwtDfNSpsjvJVJdp10p2ClQKRQiXj23EygUeWwAB1b9fOlIgHP837kq3FvQK1xiy8Ws3/tLmg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=RMY9hmf6oX/3ohGTUzigcPu/mg+by+CTQ7UM6KxUaU9tduuCqPePMxx43Ab7ByKuR9tfPgmwS6X3DCOOLIx/Im+7Ymq0vjqN3Zc6ETJMAuLbudRBl85C8IFBKYMuTCN8YsOpN1UCczcvdKAxfKzWN3gp2IfurH6jmLugj0L0OuYnKBP7NFHkmk9VG2zkVU2X65Y9FLdzZhvz1NCi9nvpjpCRdEAvax2ksZj1YWN7+dR35R+8Fc85ED9WWkCnW38oiIPeBZEF4pMILsHcch7fDHrezDY6dhQuP+AyHNE7KwLqC2Qdjj+VVTZSKL04ArED15bhHUHL/ILoaZ5u4Hw7Cg==
  • Cc: Wei Liu <wl@xxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, "Juergen Gross" <jgross@xxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 07 Mar 2024 12:20:00 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Hi Jan,

On 3/7/2024 7:04 PM, Jan Beulich wrote:
On 07.03.2024 11:38, Henry Wang wrote:
Below error can be seen when doing Yocto build of the toolstack:

| io.c: In function 'p9_error':
| io.c:684:5: error: ignoring return value of 'strerror_r' declared
   with attribute 'warn_unused_result' [-Werror=unused-result]
|   684 |     strerror_r(err, ring->buffer, ring->ring_size);
|       |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| cc1: all warnings being treated as errors

Fix the build by using strerror() to replace strerror_r(). Since
strerror() is thread-unsafe, use a separate local mutex to protect
the action. The steps would then become: Acquire the mutex first,
invoke strerror(), copy the string from strerror() to the designated
buffer and then drop the mutex.


Fixes: f4900d6d69b5 ("9pfsd: allow building with old glibc")

I will add this tag in v3.
Signed-off-by: Henry Wang <xin.wang2@xxxxxxx>
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks!

albeit there are a number of cosmetic aspects I'd have done differently
(see bottom of mail). The one thing I'd really like to ask for it a
comment ...

--- a/tools/9pfsd/io.c
+++ b/tools/9pfsd/io.c
@@ -680,8 +680,18 @@ static bool name_ok(const char *str)
  static void p9_error(struct ring *ring, uint16_t tag, uint32_t err)
  {
      unsigned int erroff;
+    static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
+    char *strerror_str;
+    RING_IDX strerror_len = 0, copy_len = 0;
+
... here explaining why strerror_r() isn't used. Unless other comments
arise and a v3 would be needed, I'd add

     /*
      * While strerror_r() exists, it comes in a POSIX and a GNU flavor.
      * Let's try to avoid trying to be clever with determining which
      * one it is that the underlying C library offers, when really we
      * don't expect this function to be called very often.
      */

while committing.

Since I am working on a V3 which will be sent out soon, please don't bother :) I will
handle it from my side.

Anyway it'll need a maintainer ack first.

+    pthread_mutex_lock(&mutex);
+    strerror_str = strerror(err);
+    strerror_len = strlen(strerror_str) + 1;
+    copy_len = min(strerror_len, ring->ring_size);
+    memcpy(ring->buffer, strerror_str, copy_len);
+    ((char *)(ring->buffer))[copy_len - 1] = '\0';
+    pthread_mutex_unlock(&mutex);
- strerror_r(err, ring->buffer, ring->ring_size);
      erroff = add_string(ring, ring->buffer, strlen(ring->buffer));
      fill_buffer(ring, P9_CMD_ERROR, tag, "SU",
                  erroff != ~0 ? ring->str + erroff : "cannot allocate memory",
     pthread_mutex_lock(&mutex);
     str = strerror(err);
     len = min(strlen(str), ring->ring_size - 1);

This actually will fire below errors on my build env, hence I separated them with a different variable.

tools/include/xen-tools/common-macros.h:38:21: error: comparison of distinct pointer types lacks a cast [-Werror]
|    38 |         (void) (&_x == &_y);                    \
|       |                     ^~
| io.c:695:11: note: in expansion of macro 'min'
|   695 |     len = min(strlen(str), MAX_ERRSTR_LEN - 1);;
|       |           ^~~
| cc1: all warnings being treated as errors

     memcpy(ring->buffer, str, len);
     ((char *)ring->buffer)[len] = '\0';
     pthread_mutex_unlock(&mutex);

I will follow your style in V3 if you don't have any specific comment on the error that I posted above (plus also not strongly disagree with my approach in v2).

Kind regards,
Henry

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.