Default Project

  • More
  • CR-9
  • finished reviewing
  • More
  • CR-108
  • finished reviewing
Do we still need this CR?

Do we still need this CR?

Why not just keep checking NewMdList != NULL? Or even ARGUMENT_PRESENT(NewMdList)

Why not just keep checking NewMdList != NULL? Or even ARGUMENT_PRESENT(NewMdList)

I tend to avoid this style. Storing the value before testing it can make it easier to debug

I tend to avoid this style. Storing the value before testing it can make it easier to debug

Did not indeed.

Did not indeed.

Yes but you can do that directly inside the FAILED() macro?

Yes but you can do that directly inside the FAILED() macro?

Isn't it better to check the return value?

Isn't it better to check the return value?

You didn't find the algorithm to compute the hash, did you?

You didn't find the algorithm to compute the hash, did you?

  • More
  • CR-108
  • finished reviewing
No need for hres?

No need for hres?

deadbeef?

deadbeef?

yay!

yay!

I added a second iteration of the patch. Feel free to have a look and review. Also make sure that you select to see the second iteration (either compared to the first iteration or base).

I added a second iteration of the patch. Feel free to have a look and review. Also make sure that you select to see the second iteration (either compared to the first iteration or base).

It's a question of risk reduction & effort required in code review. strcpy/strcat code needs to be carefully reviewed on a case-by-case basis, and if you end up having an off-by-one anywhere, you'v...

It's a question of risk reduction & effort required in code review. strcpy/strcat code needs to be carefully reviewed on a case-by-case basis, and if you end up having an off-by-one anywhere, you've got yourself a buffer overflow. If a mistake happens with strsafe code instead, it's more likely to only affect correctness and not security. As such, new code should always use strsafe and old code should be converted when convenient.

This deserves to write some tests but for now I prefer to leave this part as it was.

This deserves to write some tests but for now I prefer to leave this part as it was.

Note to self: Unregistering also needs to unregister classes from both versions.

Note to self: Unregistering also needs to unregister classes from both versions.

After thinking about it again, why? We already know that the size of the buffer is correct.

After thinking about it again, why? We already know that the size of the buffer is correct.

I'll have to inspect about it. I think we must zero the lenght when passing an atom to win32k,

I'll have to inspect about it. I think we must zero the lenght when passing an atom to win32k,

Yes, the tests show that it is.

Yes, the tests show that it is.

I think it does one more allocation that I wanted to avoid.

I think it does one more allocation that I wanted to avoid.

Noted.

Noted.

Same with beforee. It doesn't deserve the work for maintainance.

Same with beforee. It doesn't deserve the work for maintainance.

I don't think it is worth spending time to improve the readability of a hack. Eventually the default activation context will make this redundant.

I don't think it is worth spending time to improve the readability of a hack. Eventually the default activation context will make this redundant.

Noted.

Noted.

Same with before. The v5 case shouldn't matter too much as we should register for the worst case scenario. However we could skip the v6 registration if activating v6 fails.

Same with before. The v5 case shouldn't matter too much as we should register for the worst case scenario. However we could skip the v6 registration if activating v6 fails.

I think it is correct as it is. If the v6 manifest is missing it means that noone will try to create a v6 control so no need to register for it. However the v5 ones should be accessible with or wit...

I think it is correct as it is. If the v6 manifest is missing it means that noone will try to create a v6 control so no need to register for it. However the v5 ones should be accessible with or without manifests. Perhaps if the v6 is needed then THEMING_Initialize should not be called at all.

Will do.

Will do.

Noted.

Noted.