Received-SPF: pass (google.com: domain of linux-kernel+bounces-160493-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Fri, 26 Apr 2024 10:28:13 -0700 (PDT)
Subject:     Re: [PATCH 1/2] perf daemon: Fix the warning about time_t
In-Reply-To: <20240305120501.1785084-2-ben717@andestech.com>
CC: peterz@infradead.org, mingo@redhat.com, acme@kernel.org, namhyung@kernel.org,
  Mark Rutland <mark.rutland@arm.com>, alexander.shishkin@linux.intel.com, jolsa@kernel.org, irogers@google.com,
  adrian.hunter@intel.com, Paul Walmsley <paul.walmsley@sifive.com>, aou@eecs.berkeley.edu,
  linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org,
  dylan@andestech.com, tim609@andestech.com, ben717@andestech.com
From: Palmer Dabbelt <palmer@dabbelt.com>
To: ben717@andestech.com
Message-ID: <mhng-8fa791ba-8c58-44c9-a4fd-f40ab808042b@palmer-ri-x1c9>
Precedence: bulk
Mime-Version: 1.0 (MHng)
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit

On Tue, 05 Mar 2024 04:05:00 PST (-0800), ben717@andestech.com wrote:
> In the 32-bit platform, the size of time_t is still 64 bits. Thus, use
> PRIu64 to resolve the format problem.
>
> Signed-off-by: Ben Zong-You Xie <ben717@andestech.com>
> ---
>  tools/perf/builtin-daemon.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> index 83954af36753..0b6ffd51c475 100644
> --- a/tools/perf/builtin-daemon.c
> +++ b/tools/perf/builtin-daemon.c
> @@ -23,6 +23,7 @@
>  #include <sys/signalfd.h>
>  #include <sys/wait.h>
>  #include <poll.h>
> +#include <inttypes.h>
>  #include "builtin.h"
>  #include "perf.h"
>  #include "debug.h"
> @@ -688,7 +689,7 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
>  			/* lock */
>  			csv_sep, daemon->base, "lock");
>
> -		fprintf(out, "%c%lu",
> +		fprintf(out, "%c%" PRIu64 "",
>  			/* session up time */
>  			csv_sep, (curr - daemon->start) / 60);
>
> @@ -700,7 +701,7 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
>  				daemon->base, SESSION_OUTPUT);
>  			fprintf(out, "  lock:    %s/lock\n",
>  				daemon->base);
> -			fprintf(out, "  up:      %lu minutes\n",
> +			fprintf(out, "  up:      %" PRIu64 " minutes\n",
>  				(curr - daemon->start) / 60);
>  		}
>  	}
> @@ -727,7 +728,7 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
>  				/* session ack */
>  				csv_sep, session->base, SESSION_ACK);
>
> -			fprintf(out, "%c%lu",
> +			fprintf(out, "%c%" PRIu64 "",
>  				/* session up time */
>  				csv_sep, (curr - session->start) / 60);
>
> @@ -745,7 +746,7 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
>  				session->base, SESSION_CONTROL);
>  			fprintf(out, "  ack:     %s/%s\n",
>  				session->base, SESSION_ACK);
> -			fprintf(out, "  up:      %lu minutes\n",
> +			fprintf(out, "  up:      %" PRIu64 " minutes\n",
>  				(curr - session->start) / 60);
>  		}
>  	}

Sorry I missed this earlier, but IIUC this one is actually incorrect: on 
most 32-bit platforms time_t is 32 bits, it was later extended to 64 
bits.  RISC-V is special because the work to make time_t 64-bit had 
started when we submitted the port, so we just jumped straight to the 
legacy-free uABI (after some headaches).

So IIUC this would introduce a warning for some other targets.  Either 
way I shouldn't have picked it up as it's not a RISC-V patch (this is a 
generic perf file), so I'm going to drop it from fixes.

Sorry for the confusion!