linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Madhavan Srinivasan <maddy@linux.ibm.com>
To: Benjamin Gray <bgray@linux.ibm.com>,
	Kajol Jain <kjain@linux.ibm.com>,
	mpe@ellerman.id.au
Cc: atrajeev@linux.vnet.ibm.com, linuxppc-dev@lists.ozlabs.org,
	disgoel@linux.vnet.ibm.com
Subject: Re: [PATCH 2/2] selftests/powerpc/pmu: fix including of utils.h when event.h is included
Date: Tue, 7 Mar 2023 11:32:33 +0530	[thread overview]
Message-ID: <a4b97fe5-acc8-d198-92c9-cef251ca364c@linux.ibm.com> (raw)
In-Reply-To: <5dc76d52-6b22-9108-9107-281b1cd386e4@linux.ibm.com>


On 3/2/23 8:49 AM, Madhavan Srinivasan wrote:
>
> On 3/2/23 3:35 AM, Benjamin Gray wrote:
>> On Wed, 2023-03-01 at 22:39 +0530, Kajol Jain wrote:
>>> From: Madhavan Srinivasan <maddy@linux.ibm.com>
>>>
>>> event.h header already includes utlis.h. Avoid including
>>> the same explicitly in the code when event.h included.
>>>
>>> Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
>> As I understand, transitive includes should not be depended upon. If
>> you use a thing, and the thing is declared in a header, you should
>> include _that_ header. Anything else is a recipe for weird include
>> dependencies, ordering of the includes, etc.
>>
>> These files all use FAIL_IF, etc., which are declared in utils.h. So
>> utils.h is a legitimate include. The fact that events.h also includes
>> it (for u64) is a coincidence. If the u64 type def gets moved to, e.g.,
>> types.h, and utils.h is removed from events.h, suddenly all these files
>> stop compiling.
>
> thanks for the review. IIUC utils.h also carries the some test harness 
> func declarations, also some of these tests does not use type defs 
> anyway. I should have had a better commit message, my bad. But i will 
> try out the suggested case.
yeah, "utils.h" included in the testcase files are for the tast_harness 
declarations.
So we could get typedef moved from utils.h. Good catch. Thanks.
Kajol, kindly drop this patch.

Maddy
>
> Maddy

  reply	other threads:[~2023-03-07  6:03 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-01 17:09 [PATCH 1/2] selftests/powerpc/pmu: Fix sample field check in the mmcra_thresh_marked_sample_test Kajol Jain
2023-03-01 17:09 ` [PATCH 2/2] selftests/powerpc/pmu: fix including of utils.h when event.h is included Kajol Jain
2023-03-01 22:05   ` Benjamin Gray
2023-03-02  3:19     ` Madhavan Srinivasan
2023-03-07  6:02       ` Madhavan Srinivasan [this message]
2023-03-22 12:25 ` (subset) [PATCH 1/2] selftests/powerpc/pmu: Fix sample field check in the mmcra_thresh_marked_sample_test Michael Ellerman

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=a4b97fe5-acc8-d198-92c9-cef251ca364c@linux.ibm.com \
    --to=maddy@linux.ibm.com \
    --cc=atrajeev@linux.vnet.ibm.com \
    --cc=bgray@linux.ibm.com \
    --cc=disgoel@linux.vnet.ibm.com \
    --cc=kjain@linux.ibm.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    /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 NNTP newsgroup(s).