Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve SHGetFileInfo #118

Merged
merged 8 commits into from Feb 16, 2018
Merged

improve SHGetFileInfo #118

merged 8 commits into from Feb 16, 2018

Conversation

katahiromz
Copy link
Contributor

@katahiromz katahiromz commented Nov 10, 2017

This patch reduces failures of SHGetFileInfo function. CORE-7159

Purpose

Improve SHGetFileInfo.
JIRA issue: CORE-7159

TODO

  • Reduce all failures of SHGetFileInfo in shell32_winetest:shlfileop.

@katahiromz
Copy link
Contributor Author

Done. Review and commit me.

}
else
ret = SHGetFileInfoW(pathW, dwFileAttributes, NULL, sizeof(temppsfi), flags);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/sizeof(temppsfi)/0/.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@@ -429,11 +430,11 @@ DWORD_PTR WINAPI SHGetFileInfoW(LPCWSTR path,DWORD dwFileAttributes,
return FALSE;

/* windows initializes these values regardless of the flags */
if (psfi != NULL)
if (psfi)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid unnecessary style changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@@ -566,8 +591,8 @@ DWORD_PTR WINAPI SHGetFileInfoW(LPCWSTR path,DWORD dwFileAttributes,

if (flags & SHGFI_LINKOVERLAY)
uGilFlags |= GIL_FORSHORTCUT;
else if ((flags&SHGFI_ADDOVERLAYS) ||
(flags&(SHGFI_ICON|SHGFI_SMALLICON))==SHGFI_ICON)
else if ((flags & SHGFI_ADDOVERLAYS) ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More style changes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@binarymaster
Copy link
Member

Is this PR ready to squash & merge?

@yagoulas
Copy link
Member

I think so. One thing that I still wonder about is if the fixes are supposed to be so simple, why didn't wine fix them already.

{
if (flags & SHGFI_USEFILEATTRIBUTES)
{
return 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some magic value here.

Is it SHGFI_SMALLICON (0x000000001)?

@katahiromz
Copy link
Contributor Author

SHGetFileInfo function
https://msdn.microsoft.com/en-us/library/windows/desktop/bb762179.aspx

In this case, SHGFI_USEFILEATTRIBUTES and SHGFI_EXETYPE flags are used. MSDN says SHGFI_USEFILEATTRIBUTES and SHGFI_EXETYPE are mutually exclusive. So this case is abnormal. I don't know why it is TRUE.

@katahiromz
Copy link
Contributor Author

SHGFI_SMALLICON is not for the return value. Can anyone understand?

@learn-more
Copy link
Member

@katahiromz I do not understand your question.

@katahiromz
Copy link
Contributor Author

SHGFI_SMALLICON is usually used as a flag. It is not used as the return value. The test result says using TRUE as a return value in this case. Okay?

@learn-more
Copy link
Member

Probably because both are defined as 1?

#define SHGFI_SMALLICON 1

@katahiromz
Copy link
Contributor Author

Yep.

@gedmurphy
Copy link
Member

@yagoulas @gigaherz @learn-more unless you object, I'll merge them this weekend

@HBelusca
Copy link
Contributor

@katahiromz : are there other things to add to this patch or is it ok to be merged?

@katahiromz
Copy link
Contributor Author

@HBelusca: It's ready to be merged.

@gedmurphy gedmurphy merged commit 338799b into reactos:master Feb 16, 2018
julenuri pushed a commit to julenuri/reactos that referenced this pull request Aug 16, 2021
* Added an entry for Dave Gnukem
Dave Gnukem is a retro-style 2D scrolling platform shooter similar to, and inspired by, Duke Nukem 1
The source and assets are licensed under MIT GPL and CC
https://github.com/davidjoffe/dave_gnukem

I tested the download and install with RAPPS and it worked.
This game installed and ran perfectly even with sound using ReactOS 0.4.13

The creator of the game made some comments in his repo about making changes to the gmae so that it
could run under ReactOS.

This is the latest version

* added extra licenses
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants