From: Nick Clifton <nickc@redhat.com>
To: Indu Bhagat <indu.bhagat@oracle.com>, binutils@sourceware.org
Cc: weimin.pan@oracle.com
Subject: Re: [PATCH 3/6] sframe: gas: libsframe: define constants and remove magic numbers
Date: Thu, 8 Dec 2022 11:10:40 +0000 [thread overview]
Message-ID: <3f2c38f1-e08c-1584-2e4c-77b4eb50351d@redhat.com> (raw)
In-Reply-To: <20221207195222.1182788-4-indu.bhagat@oracle.com>
Hi Indu,
> +#define SFRAME_FRE_TYPE_ADDR1_LIMIT ((SFRAME_FRE_TYPE_ADDR1+1)*8)
For readabilities sake, I would recommend adding whitespace around arithmetic
operations. For example in the define above a quick glance would suggest that
the definition is for a symbol called ...ADDR11 rather than ...ADDR1 + 1.
So:
#define SFRAME_FRE_TYPE_ADDR1_LIMIT ((SFRAME_FRE_TYPE_ADDR1 + 1) * 8)
Is better IMHO.
> +#define SFRAME_FRE_TYPE_ADDR2_LIMIT ((SFRAME_FRE_TYPE_ADDR2*2)*8)
> +#define SFRAME_FRE_TYPE_ADDR4_LIMIT ((SFRAME_FRE_TYPE_ADDR4*2)*8)
The same goes for these two definitions as well.
Patch approved with these changes.
Cheers
Nick
PS. Just checking, since I am not actually familiar with the sframe
format: Is it correct that SFRAME_FRE_TYPE_ADDR1_LIMIT is defined
as "(...ADDR1 + 1) * 8" rather than "(...ADDR1 * 2) * 8)" ? It is
just that the other two limits are defined using the second formula
and it seems slightly odd that it is not used for the first.
next prev parent reply other threads:[~2022-12-08 11:10 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-07 19:52 [PATCH 0/6] Small improvements around SFrame support Indu Bhagat
2022-12-07 19:52 ` [PATCH 1/6] libsframe: minor formatting nits Indu Bhagat
2022-12-08 11:03 ` Nick Clifton
2022-12-07 19:52 ` [PATCH 2/6] sframe.h: make some macros more precise Indu Bhagat
2022-12-07 23:52 ` Hans-Peter Nilsson
2022-12-08 17:36 ` Indu Bhagat
2022-12-08 20:26 ` [PATCH,V2 " Indu Bhagat
2022-12-12 16:02 ` Nick Clifton
2022-12-07 19:52 ` [PATCH 3/6] sframe: gas: libsframe: define constants and remove magic numbers Indu Bhagat
2022-12-08 11:10 ` Nick Clifton [this message]
2022-12-08 17:43 ` Indu Bhagat
2022-12-08 18:38 ` Indu Bhagat
2022-12-07 19:52 ` [PATCH 4/6] gas: sframe: fine tune the fragment fixup for SFrame func info Indu Bhagat
2022-12-08 11:12 ` Nick Clifton
2022-12-07 19:52 ` [PATCH 5/6] libsframe: rename API sframe_fde_func_info to sframe_fde_create_func_info Indu Bhagat
2022-12-08 11:13 ` Nick Clifton
2022-12-07 19:52 ` [PATCH 6/6] objdump: sframe: fix memory leaks Indu Bhagat
2022-12-08 11:14 ` Nick Clifton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3f2c38f1-e08c-1584-2e4c-77b4eb50351d@redhat.com \
--to=nickc@redhat.com \
--cc=binutils@sourceware.org \
--cc=indu.bhagat@oracle.com \
--cc=weimin.pan@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).