Re: [PATCH 9/9] malloc_info.3: Use intptr_t to store pointers

["Alejandro Colomar (man-pages)" <alx.manpages@gmail.com>]

Hi Stephen,

On 1/7/22 17:46, Stephen Kitt wrote:
> int isn't large enough to store pointers on all platforms, use
> intptr_t instead.

Well, since the pointer came from a previous 'int', there should be no 
problem.  But since the C language (or even POSIX) is very permissive 
about what a conforming implementation can do with pointers, and it only 
guarantees conversions to/from [u]intptr_t, I'd take this patch for 
correctness.  However...

I still don't like at all the fact that compilers require an explicit 
cast.  Casts are dangerous.  Period.  I haven't seen a place where casts 
are good, except when you explicitly want to rely on undefined behavior 
(which I've found very useful in a few cases).  It should be clear to 
the compiler that we're doing the right thing, since we declared the 
variable to be 'intptr_t', which is by itself very rare.  Requiring a 
cast adds no clarity, while it does increase the chances of a bug.

I'd like to know your opinion.  I think I'll remove the cast, even if 
compilers are going to give a warning for implicitly converting from 
pointer to integer (intptr_t).  I'd say patch the compiler, not the code.

Thanks,

Alex

> 
> Signed-off-by: Stephen Kitt <steve@sk2.org>
> ---
>   man3/malloc_info.3 | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/man3/malloc_info.3 b/man3/malloc_info.3
> index a5b8d34f9..3baa891fd 100644
> --- a/man3/malloc_info.3
> +++ b/man3/malloc_info.3
> @@ -198,7 +198,7 @@ static int numThreads, numBlocks;
>   static void *
>   thread_func(void *arg)
>   {
> -    int tn = (int) arg;
> +    intptr_t tn = (intptr_t) arg;
>   
>       /* The multiplier \(aq(2 + tn)\(aq ensures that each thread (including
>          the main thread) allocates a different amount of memory. */
> @@ -237,7 +237,7 @@ main(int argc, char *argv[])
>   
>       /* Create threads that allocate different amounts of memory. */
>   
> -    for (int tn = 0; tn < numThreads; tn++) {
> +    for (intptr_t tn = 0; tn < numThreads; tn++) {
>           errno = pthread_create(&thr[tn], NULL, thread_func,
>                                  (void *) tn);
>           if (errno != 0)

-- 
Alejandro Colomar
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/