Doron
Fri Sep 09 01:39:46 CDT 2005
i never said it was the perfect assumption to make, i just gave the history
behind it. we all learn from our mistakes, move on and correct and try not
to repeat them in the future. we are not perfect ;).
d
--
Please do not send e-mail directly to this alias. this alias is for
newsgroup purposes only.
This posting is provided "AS IS" with no warranties, and confers no rights.
"Calvin Guan" <hguan@nospam.broadcom.com> wrote in message
news:uP1J9FFtFHA.420@TK2MSFTNGP15.phx.gbl...
> Sorry, Doron. They are making too many assumptions. Writing macros with
> side effect is evil. This certainly traps experienced developers too. Why?
> We have more than enough to worry in our driver and h/w. For such
> seemingly trivial service, I think most of us just trust it does what it's
> supposed to until we got burnt at some point later, like, why the hell my
> nic is not sending. After you traced a big chunk of code and finally
> discovered it's because of using someone's macro with side effect. I'm
> sure one would get pissed off. Especially in such case, it smells like a
> function.
>
> Just my $0.02 worth.
>
> --
> Calvin Guan (Windows DDK MVP)
> NetXtreme Longhorn Miniport Prime
> Broadcom Corp. www.broadcom.com
>
> "Doron Holan [MS]" <doronh@nospam.microsoft.com> wrote in message
> news:%23D%23h%23cEtFHA.4076@TK2MSFTNGP11.phx.gbl...
>> the folks who put things in these headers are experienced, but you have
>> to understand the purpose behind these additions and others like it.
>> they are assuming that they are the primary consumers of these
>> macros/functions and that they will be used properly and that the user
>> will look at them before using them. this doesn't scale well and
>> certainly traps newbies, but until recently, driver work has been outside
>> of the domain of a newbie to begin with.
>>
>> this is being addressed. for instance, InitializeListHead and friends
>> have been macros until the server sp1 DDK. the old definition
>>
>> #define InitializeListHead(ListHead) (\
>> (ListHead)->Flink = (ListHead)->Blink = (ListHead))
>>
>> and the new one (which is safer and much easier to debug b/c you can
>> actually step into it)
>>
>> VOID
>> FORCEINLINE
>> InitializeListHead(
>> IN PLIST_ENTRY ListHead
>> )
>> {
>> ListHead->Flink = ListHead->Blink = ListHead;
>> }
>>
>> RtlRetrieveXxx could benefit from same conversion to FORCEINLINE, I will
>> suggest that to the relevant folks.
>>
>> d
>>
>>
>> --
>> Please do not send e-mail directly to this alias. this alias is for
>> newsgroup purposes only.
>> This posting is provided "AS IS" with no warranties, and confers no
>> rights.
>>
>>
>> "Norman Diamond" <ndiamond@community.nospam> wrote in message
>> news:uLeQ6uAtFHA.3000@TK2MSFTNGP12.phx.gbl...
>>> For two more interview questions, find two more bugs in the same macros.
>>>
>>> Answers:
>>>
>>> (1) If callers don't wrap the argument for DEST_ADDRESS in parentheses
>>> then
>>> even worse evil can happen.
>>>
>>> #include <ntddk.h>
>>> void foo( void ) {
>>> UCHAR x[] = { 1,2,3,4,5,6,7,8,9,0 };
>>> PUCHAR p = &x[1];
>>> USHORT y[5];
>>> RtlRetrieveUshort( y + 1, p ); // stores into one byte of y[0]
>>> // and one byte of y[1]
>>>
>>> (2) MSDN documents them as routines instead of macros. Programs that
>>> rely
>>> on MSDN's contract can use expressions that have side effects (such as
>>> p++)
>>> in arguments, and then discover that MSDN needs more fixes.
>>>
>>> I wonder what would happen if Microsoft assigned some experienced C
>>> programmers to fix bugs in the DDK.
>>>
>>>
>>> "Pavel A." <pavel_a@NOwritemeNO.com> wrote in message
>>> news:B68C6A01-16C2-45D2-A103-33691AF1B1C5@microsoft.com...
>>>> Here's a good interview question...
>>>> Find a bug in RtlRetrieveUshort/Ulong macros in ntddk.h
>>>> (DDK 3790.1830 / wnet)
>>>>
>>>> For x86 RtlRetrieveUshort, RtlRetrieveUlong are defined as:
>>>>
>>>> #define RtlRetrieveUshort(DEST_ADDRESS,SRC_ADDRESS) \
>>>> if ((ULONG_PTR)SRC_ADDRESS & SHORT_MASK)
>>>> { \
>>>> ((PUCHAR) DEST_ADDRESS)[0] = ((PUCHAR) SRC_ADDRESS)[0]; \
>>>> ((PUCHAR) DEST_ADDRESS)[1] = ((PUCHAR) SRC_ADDRESS)[1]; \
>>>> } \
>>>> else { \
>>>> *((PUSHORT) DEST_ADDRESS) = *((PUSHORT) SRC_ADDRESS); \
>>>> } \
>>>>
>>>>
>>>> Answer: SRC_ADDRESS must be enclosed in braces,
>>>> or evil will happen. How to repro:
>>>>
>>>> #include <ntddk.h>
>>>> void foo( void ) {
>>>> UCHAR x[] = { 1,2,3,4,5,6,7,8,9,0 };
>>>> PUCHAR p = &x[1];
>>>> USHORT y;
>>>>
>>>> RtlRetrieveUshort( &y, p + 6 ); // fails
>>>> RtlRetrieveUshort( &y, p + 1 ); // works
>>>> }
>>>>
>>>> Same with RtlRetrieveUlong.
>>>>
>>>> Regards,
>>>> --PA
>>>>
>>>
>>
>>
>
>