Re: [ros-dev] [ros-diffs] [akhaldi] 71183: [CLASSPNP] Fix MSVC build. Brought to you by Timo.

[Alex Ionescu]

Thomas, the standard says:

"If one of the strings has an encoding prefix and the other doesn't,
the one that doesn't will be considered to have the same encoding
prefix as the other."

In other words, this now sends a wide-string to ASSERTMSG. Which is wrong.
Best regards,
Alex Ionescu

On Sun, Apr 24, 2016 at 5:14 AM, Thomas Faber [e-mail] wrote:
> On 2016-04-20 14:16, [e-mail] wrote:
> > [CLASSPNP] Fix MSVC build. Brought to you by Timo.
> > --- trunk/reactos/drivers/storage/classpnp/class.c    [iso-8859-1] (original)
> > +++ trunk/reactos/drivers/storage/classpnp/class.c    [iso-8859-1] Wed Apr 20 12:16:16 2016
> > @@ -877,7 +877,7 @@
> >                  status = devInfo->ClassStopDevice(DeviceObject, IRP_MN_STOP_DEVICE);
> >              ASSERTMSG("ClassDispatchPnp !! STOP_DEVICE should "
> >                      "never be failed\n", NT_SUCCESS(status));
> >                      L"never be failed\n", NT_SUCCESS(status));
> >
> >            if(isFdo) {
> >                status = ClassForwardIrpSynchronous(commonExtension, Irp);
>
> This makes no sense to me at all. You're now concatenating a narrow and
> a wide string (which should be an error, but compilers suck), and
> passing it to a macro that calls RtlAssert -- which expects a narrow
> string.
> Am I missing something here?
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[Timo Kreuzer]

Guys, are you incapable of reading code, or something? At least in case 
of Alex, it cannot be the missing time, since you obviously had the time 
to find and copy a section of the standard.

The "patch" I gave to Amine actually contained ASSERTMSGW and 2 L 
prefixes. Might have got lost in translation. The current version is not 
beautiful, but it works correctly! I leave it to the reader to figure it 
out!

Btw, the reason for this hack was that VS versions before VS 2015 do not 
follow the standard and error out, if you concatenate 2 strings with 
different encoding. This breaks the NT_ASSERTMSG macro from latest WDK.

Next time, you make bold statements about something being broken, you 
should take at least a minimum amount of time to investigate the issue!

Thanks,
Timo

PS: For anyone who doesn't know how to find code: http://tinyurl.com/lmftcfy

Am 24.04.2016 um 17:19 schrieb Alex Ionescu:
> Thomas, the standard says:
>
> "If one of the strings has an encoding prefix and the other doesn't,
> the one that doesn't will be considered to have the same encoding
> prefix as the other."
>
> In other words, this now sends a wide-string to ASSERTMSG. Which is wrong.
> Best regards,
> Alex Ionescu
>
> On Sun, Apr 24, 2016 at 5:14 AM, Thomas Faber [e-mail] wrote:
> > On 2016-04-20 14:16, [e-mail] wrote:
> > > [CLASSPNP] Fix MSVC build. Brought to you by Timo.
> > > --- trunk/reactos/drivers/storage/classpnp/class.c    [iso-8859-1] (original)
> > > +++ trunk/reactos/drivers/storage/classpnp/class.c    [iso-8859-1] Wed Apr 20 12:16:16 2016
> > > @@ -877,7 +877,7 @@
> > >                   status = devInfo->ClassStopDevice(DeviceObject, IRP_MN_STOP_DEVICE);
> > >               ASSERTMSG("ClassDispatchPnp !! STOP_DEVICE should "
> > >                      "never be failed\n", NT_SUCCESS(status));
> > >                      L"never be failed\n", NT_SUCCESS(status));
> > >
> > >             if(isFdo) {
> > >                 status = ClassForwardIrpSynchronous(commonExtension, Irp);
> >
> > This makes no sense to me at all. You're now concatenating a narrow and
> > a wide string (which should be an error, but compilers suck), and
> > passing it to a macro that calls RtlAssert -- which expects a narrow
> > string.
> > Am I missing something here?
> >
> > Ros-dev mailing list
> > [e-mail]
> > http://www.reactos.org/mailman/listinfo/ros-dev
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev