[ros-dev] [ros-diffs] [sginsberg] 43265: - Replace some x86 assembly in drivers with portable breakpoint support.

[Alex Ionescu]

Portbability is ALWAYS needed, except I guess to people like who you
insist on using assembly even when it's been proven to be unreliable,
unportable and inefficient. How is the PPC build or ARM build supposed
to work with your component?

And why the fuck can't you be bothered to use "__break()"??? It's
inlined -- producing exactly the same int 3 code as you were using on
x86, would've saved you to have to re-define DbgBreakPoint, and
would've been portable from day one.

Best regards,
Alex Ionescu



On Sat, Oct 3, 2009 at 6:03 PM, Timo Kreuzer <timo.kreuzer at web.de> wrote:
>
> It was intentionally that I used an inlined break point in bmfd & ftfd.
> Portability is not yet needed. Please revert.
>
>
> sginsberg at svn.reactos.org wrote:
>> Author: sginsberg
>> Date: Sat Oct  3 16:15:46 2009
>> New Revision: 43265
>>
>> URL: http://svn.reactos.org/svn/reactos?rev=43265&view=rev
>> Log:
>> - Replace some x86 assembly in drivers with portable breakpoint support.
>>
>> Modified:
>>     trunk/reactos/drivers/network/ndis/ndis/miniport.c
>>     trunk/reactos/drivers/video/font/bmfd/bmfd.h
>>     trunk/reactos/drivers/video/font/bmfd/font.c
>>     trunk/reactos/drivers/video/font/bmfd/glyph.c
>>     trunk/reactos/drivers/video/font/ftfd/enable.c
>>     trunk/reactos/drivers/video/font/ftfd/font.c
>>     trunk/reactos/drivers/video/font/ftfd/ftfd.h
>>
>> Modified: trunk/reactos/drivers/network/ndis/ndis/miniport.c
>> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/ndis/ndis/miniport.c?rev=43265&r1=43264&r2=43265&view=diff
>> ==============================================================================
>> --- trunk/reactos/drivers/network/ndis/ndis/miniport.c [iso-8859-1] (original)
>> +++ trunk/reactos/drivers/network/ndis/ndis/miniport.c [iso-8859-1] Sat Oct  3 16:15:46 2009
>> @@ -1472,7 +1472,7 @@
>>    *NdisWrapperHandle = NULL;
>>
>>  #if BREAK_ON_MINIPORT_INIT
>> -  __asm__ ("int $3\n");
>> +  DbgBreakPoint();
>>  #endif
>>
>>    Miniport = ExAllocatePool(NonPagedPool, sizeof(NDIS_M_DRIVER_BLOCK));
>>
>> Modified: trunk/reactos/drivers/video/font/bmfd/bmfd.h
>> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/video/font/bmfd/bmfd.h?rev=43265&r1=43264&r2=43265&view=diff
>> ==============================================================================
>> --- trunk/reactos/drivers/video/font/bmfd/bmfd.h [iso-8859-1] (original)
>> +++ trunk/reactos/drivers/video/font/bmfd/bmfd.h [iso-8859-1] Sat Oct  3 16:15:46 2009
>> @@ -263,17 +263,6 @@
>>  ULONG
>>  DbgPrint(IN PCHAR Format, IN ...);
>>
>> -FORCEINLINE
>> -VOID
>> -DbgBreakPoint(VOID)
>> -{
>> -#ifdef __GNUC__
>> -    asm volatile ("int $3");
>> -#else
>> -   __asm int 3;
>> -#endif
>> -}
>> -
>>  DHPDEV
>>  APIENTRY
>>  BmfdEnablePDEV(
>>
>> Modified: trunk/reactos/drivers/video/font/bmfd/font.c
>> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/video/font/bmfd/font.c?rev=43265&r1=43264&r2=43265&view=diff
>> ==============================================================================
>> --- trunk/reactos/drivers/video/font/bmfd/font.c [iso-8859-1] (original)
>> +++ trunk/reactos/drivers/video/font/bmfd/font.c [iso-8859-1] Sat Oct  3 16:15:46 2009
>> @@ -247,7 +247,7 @@
>>      ULONG cjView;
>>
>>      DbgPrint("BmfdLoadFontFile()\n");
>> -    DbgBreakPoint();
>> +    EngDebugBreak();
>>
>>      /* Check parameters */
>>      if (cFiles != 1)
>> @@ -323,7 +323,7 @@
>>      PBMFD_FILE pfile = (PBMFD_FILE)iFile;
>>
>>      DbgPrint("BmfdQueryFontFile()\n");
>> -//    DbgBreakPoint();
>> +//    EngDebugBreak();
>>
>>      switch (ulMode)
>>      {
>> @@ -397,7 +397,7 @@
>>      HGLYPH * phglyphs;
>>
>>      DbgPrint("DrvQueryFontTree(iMode=%ld)\n", iMode);
>> -//    DbgBreakPoint();
>> +//    EngDebugBreak();
>>
>>      /* Check parameters, we only support QFT_GLYPHSET */
>>      if (!iFace || iFace > pfile->cNumFaces || iMode != QFT_GLYPHSET)
>> @@ -522,7 +522,7 @@
>>      PANOSE panose = {0};
>>
>>      DbgPrint("BmfdQueryFont()\n");
>> -//    DbgBreakPoint();
>> +//    EngDebugBreak();
>>
>>      /* Validate parameters */
>>      if (iFace > pfile->cNumFaces || !pid)
>>
>> Modified: trunk/reactos/drivers/video/font/bmfd/glyph.c
>> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/video/font/bmfd/glyph.c?rev=43265&r1=43264&r2=43265&view=diff
>> ==============================================================================
>> --- trunk/reactos/drivers/video/font/bmfd/glyph.c [iso-8859-1] (original)
>> +++ trunk/reactos/drivers/video/font/bmfd/glyph.c [iso-8859-1] Sat Oct  3 16:15:46 2009
>> @@ -331,7 +331,7 @@
>>
>>      DbgPrint("BmfdQueryFontData(pfo=%p, iMode=%ld, hg=%p, pgd=%p, pv=%p, cjSize=%ld)\n",
>>               pfo, iMode, hg, pgd, pv, cjSize);
>> -//    DbgBreakPoint();
>> +//    EngDebugBreak();
>>
>>      switch (iMode)
>>      {
>>
>> Modified: trunk/reactos/drivers/video/font/ftfd/enable.c
>> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/video/font/ftfd/enable.c?rev=43265&r1=43264&r2=43265&view=diff
>> ==============================================================================
>> --- trunk/reactos/drivers/video/font/ftfd/enable.c [iso-8859-1] (original)
>> +++ trunk/reactos/drivers/video/font/ftfd/enable.c [iso-8859-1] Sat Oct  3 16:15:46 2009
>> @@ -77,7 +77,7 @@
>>      IN HANDLE hDriver)
>>  {
>>      DbgPrint("FtfdEnablePDEV(hdev=%p)\n", hdev);
>> -    DbgBreakPoint();
>> +    EngDebugBreak();
>>
>>
>>      /* Return a dummy DHPDEV */
>>
>> Modified: trunk/reactos/drivers/video/font/ftfd/font.c
>> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/video/font/ftfd/font.c?rev=43265&r1=43264&r2=43265&view=diff
>> ==============================================================================
>> --- trunk/reactos/drivers/video/font/ftfd/font.c [iso-8859-1] (original)
>> +++ trunk/reactos/drivers/video/font/ftfd/font.c [iso-8859-1] Sat Oct  3 16:15:46 2009
>> @@ -145,7 +145,7 @@
>>      PFTFD_FILE pfile = (PFTFD_FILE)iFile;
>>
>>      DbgPrint("FtfdQueryFontFile(ulMode=%ld)\n", ulMode);
>> -//    DbgBreakPoint();
>> +//    EngDebugBreak();
>>
>>      switch (ulMode)
>>      {
>> @@ -322,7 +322,7 @@
>>      FT_Done_Face(ftface);
>>
>>      DbgPrint("Finished with the ifi: %p\n", pifiX);
>> -    DbgBreakPoint();
>> +    EngDebugBreak();
>>
>>      return pifi;
>>  }
>> @@ -469,7 +469,7 @@
>>      *pid = (ULONG_PTR)pGlyphSet;
>>
>>  DbgPrint("pGlyphSet=%p\n", pGlyphSet);
>> -DbgBreakPoint();
>> +EngDebugBreak();
>>
>>      return pGlyphSet;
>>  }
>>
>> Modified: trunk/reactos/drivers/video/font/ftfd/ftfd.h
>> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/video/font/ftfd/ftfd.h?rev=43265&r1=43264&r2=43265&view=diff
>> ==============================================================================
>> --- trunk/reactos/drivers/video/font/ftfd/ftfd.h [iso-8859-1] (original)
>> +++ trunk/reactos/drivers/video/font/ftfd/ftfd.h [iso-8859-1] Sat Oct  3 16:15:46 2009
>> @@ -51,17 +51,6 @@
>>
>>  ULONG
>>  DbgPrint(IN PCHAR Format, IN ...);
>> -
>> -FORCEINLINE
>> -VOID
>> -DbgBreakPoint(VOID)
>> -{
>> -#ifdef __GNUC__
>> -    asm volatile ("int $3");
>> -#else
>> -   __asm int 3;
>> -#endif
>> -}
>>
>>  DHPDEV
>>  APIENTRY
>>
>>
>>
>>
>
>
> _______________________________________________
> Ros-dev mailing list
> Ros-dev at reactos.org
> http://www.reactos.org/mailman/listinfo/ros-dev
>