Dashboard

  • More
  • CR-66
  • summarized and closed
The change to strlen should also not be necessary. -1 does that

The change to strlen should also not be necessary. -1 does that

Wine won't accept style-only changes anyway so there's no point to doing either

Wine won't accept style-only changes anyway so there's no point to doing either

If we are changing this, might as well add spaces around the &

If we are changing this, might as well add spaces around the &

Can we test this?

Can we test this?

This should be covered by the test.

This should be covered by the test.

INVALID_FILE_ATTRIBUTES

INVALID_FILE_ATTRIBUTES

Undefined behaviour if psfParent is NULL ?

Undefined behaviour if psfParent is NULL ?

This is prone to buffer overflows, it should contain the size of the buffer, not the size of the string.

This is prone to buffer overflows, it should contain the size of the buffer, not the size of the string.

Should we fail and return here if we can't bind to parent shellfolder instead of continuing here ?

Should we fail and return here if we can't bind to parent shellfolder instead of continuing here ?

Why 0 in case we don't have psfi became sizeof(temppfsi) ?

Why 0 in case we don't have psfi became sizeof(temppfsi) ?

shell32: Fix some bugs of SHGetFileInfoA/W function
shell32: Fix some bugs of SHGetFileInfoA/W function
  • More
  • CR-110
  • resumed reviewing
  • More
  • CR-108
  • finished reviewing
If you need this for debugging, please indicate where you need it and how it would help. The current amount of spam in this file is already tremendous when uncommenting NDEBUG, and it makes output ...

If you need this for debugging, please indicate where you need it and how it would help.
The current amount of spam in this file is already tremendous when uncommenting NDEBUG, and it makes output already completely useless imo.

Will do in a separate commit.

Will do in a separate commit.

With an @ prefix is not found. I suspect that the @ prefix is only used when there are 2 fonts with the same face name in one package, where one font is vertical.

With an @ prefix is not found.
I suspect that the @ prefix is only used when there are 2 fonts with the same face name in one package, where one font is vertical.

Misconfigured editor, I'll fix it probably in the week-end https://code.reactos.org/static/olpro3/2static/images/wiki/icons/emoticons/tongue.gif (yeah I'm busy with life right now)

Misconfigured editor, I'll fix it probably in the week-end (yeah I'm busy with life right now)

Would returning a failure code be sufficient ? I fear a bogous index could be forged in userland (attacker, misbehaving app, etc) and we may want to fail gracefully instead (unless sanity checks I ...

Would returning a failure code be sufficient ? I fear a bogous index could be forged in userland (attacker, misbehaving app, etc) and we may want to fail gracefully instead (unless sanity checks I don't remember also occur before, in this case it's OK for asserting)

Has No3Ofin1V vertical layout (@No3Ofin1V)? Check it out.

Has No3Ofin1V vertical layout (@No3Ofin1V)? Check it out.

NTSTATUS NTAPI suggests that this can be called from external code as well, it might make sense to assert on Index being in an acceptable range, since we index an array with it.

NTSTATUS NTAPI suggests that this can be called from external code as well,
it might make sense to assert on Index being in an acceptable range, since we index an array with it.

We don't care if this fails?

We don't care if this fails?

It looks like WdmAudCleanup will do that for us

It looks like WdmAudCleanup will do that for us

This cast does nothing, it's already the same type

This cast does nothing, it's already the same type

Maybe we should link it to the MixerId or the MixerInfo structure in some way

Maybe we should link it to the MixerId or the MixerInfo structure in some way

This is a strange way to identify an event. ... but I don't have a better suggestion, so I guess this is fine for now.

This is a strange way to identify an event.
... but I don't have a better suggestion, so I guess this is fine for now.

Heh, I didn't notice this was disabled. So disregard the comment above – however a FIXME comment as a reminder to change it back would be good.

Heh, I didn't notice this was disabled. So disregard the comment above – however a FIXME comment as a reminder to change it back would be good.

This doesn't need a cast

This doesn't need a cast