Macro inflation [was: Re: change log for rtems (2010-09-08)]

Sebastian Huber sebastian.huber at embedded-brains.de
Fri Sep 10 07:18:05 UTC 2010


On 09/10/2010 08:45 AM, Thomas Dörfler wrote:
> Eric, Ralf,
> 
> obviously we have different opinions here. Once again: I agree that a
> different namespace is required for these definitions, but I think the
> macros _do_ improve readability.
> 
> I would like to move the discussion back to the rtems-vc mailing list to
> allow Sebastian to express his opinions.
> 
> wkr,
> 
> Thomas.
> 
> On 09.09.2010 15:43, Eric Norum wrote:
>> On Sep 9, 2010, at 2:40 AM, Ralf Corsepius wrote:
>>
>>> Hi,
>>>
>>> Sebastian Huber committed the patch to CVS, yesterday. Unfortunately nobody so far has responded to my remark.
>>>
>>> Therefore, I don't see another alternative but to bring this to the attention of the SC.
>>>
>>> Rationale:
>>>
>>> a) this patch is bad code and must be removed.
>>> The macros are not namespace safe and thus are likely to clash with other macros. E.g. using a macro called BIT8 is grossly negligant.

Yes, it is not name space safe.  There is no reason for this.  This file is
intended to simplify the description of flags and fields from hardware
registers.  These definitions are only useful for a limited scope.  If you put
the hardware dependent definitions into a wider scope, e.g. via bsp.h you do
something wrong.

Also preprocessor stuff doesn't show up in the symbol table.

You can find such macros everywhere in the source code.  They are nothing new.
 It is only a collection of them in a separate header file.

>>>
>>> b) I fundamentally question the usefulness and the correctness of this file and consider this code to be dirty.

If you don't like this header file, don't use it.

>>>
>>>
>>> Ralf
>>
>>
>> I'm 100% in agreement with Ralf here.
>> Namespace pollution is a serious issue.
>> The macros don't do anything to improve readability of the code in which they are used.  

Making it really easy to write, read and verify flag and field definitions for
big-endian machines from Motorola and Freescale manuals is not an improvement?

>> If someone doesn't understand the C idioms for bit-set, bit-test, bit-clear, etc. they probably shouldn't be trying to program device I/O routines.

Do you claim that someone who uses such macros doesn't understand the C idioms
for bit-set, bit-test, bit-clear, etc.?

Example (MPC5200 - ICTL External Enable and External Types Register):

#define ICTL_EET_ECLR0 BBIT32(4)
#define ICTL_EET_ECLR1 BBIT32(5)
#define ICTL_EET_ECLR2 BBIT32(6)
#define ICTL_EET_ECLR3 BBIT32(7)
#define ICTL_EET_ETYPE0(val) BFIELD32(val, 8, 9)
#define ICTL_EET_ETYPE1(val) BFIELD32(val, 10, 11)
#define ICTL_EET_ETYPE2(val) BFIELD32(val, 12, 13)
#define ICTL_EET_ETYPE3(val) BFIELD32(val, 14, 15)
#define ICTL_EET_SET_ETYPE0(reg, val) BSETFIELD32(reg, val, 8, 9)
#define ICTL_EET_SET_ETYPE1(reg, val) BSETFIELD32(reg, val, 10, 11)
#define ICTL_EET_SET_ETYPE2(reg, val) BSETFIELD32(reg, val, 12, 13)
#define ICTL_EET_SET_ETYPE3(reg, val) BSETFIELD32(reg, val, 14, 15)
#define ICTL_EET_MEE BBIT32(19)
#define ICTL_EET_EENA0 BBIT32(20)
#define ICTL_EET_EENA1 BBIT32(21)
#define ICTL_EET_EENA2 BBIT32(22)
#define ICTL_EET_EENA3 BBIT32(23)
#define ICTL_EET_CEB BBIT32(31)

Please write this without these macros and compare the results.

-- 
Sebastian Huber, embedded brains GmbH

Address : Obere Lagerstr. 30, D-82178 Puchheim, Germany
Phone   : +49 89 18 90 80 79-6
Fax     : +49 89 18 90 80 79-9
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.



More information about the vc mailing list