Re: lib: include rte_memory.h for __rte_cache_aligned

Hi Neil,

2014-12-10 19:28, Neil Horman:

On Wed, Dec 10, 2014 at 07:09:03PM +0000, Jia Yu wrote:

quoted

Hi Neil,

Moving __rte_cache_aligned right after struct keyword will help. On the
other hand, enforcing this rule for existing (100+) and future definitions
will be difficult. It¹s clearer and a good practice to include header file
explicitly.

You need to include the header file regardless of what you do.  The advantage to
placing the macro right after the struct keyword is that failure to include the
header will result in a compiler error, rather than silent behavioral changes
and run time breakage.

You don't have to enforce putting the attribute after the struct keyword, I'd
say just move them now to protect the current code.  Subsequent patch authors
will see the existing style and follow suit.  Or they won't, and we'll point out
the issue during review.

It should be a different patch for next release cycle.
Let's apply this fix only for 1.8.0.

-- 
Thomas

On Thu, Dec 11, 2014 at 01:36:54AM +0100, Thomas Monjalon wrote:

Hi Neil,

2014-12-10 19:28, Neil Horman:

quoted

On Wed, Dec 10, 2014 at 07:09:03PM +0000, Jia Yu wrote:

quoted

Hi Neil,

Moving __rte_cache_aligned right after struct keyword will help. On the
other hand, enforcing this rule for existing (100+) and future definitions
will be difficult. It¹s clearer and a good practice to include header file
explicitly.

You need to include the header file regardless of what you do.  The advantage to
placing the macro right after the struct keyword is that failure to include the
header will result in a compiler error, rather than silent behavioral changes
and run time breakage.

You don't have to enforce putting the attribute after the struct keyword, I'd
say just move them now to protect the current code.  Subsequent patch authors
will see the existing style and follow suit.  Or they won't, and we'll point out
the issue during review.

It should be a different patch for next release cycle.
Let's apply this fix only for 1.8.0.

Why?  Theres no harm in doing so now.
Neil