Skip to content

win32: Fix enumerate root dir content on SFTP-server #148

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

Merged
merged 3 commits into from
May 26, 2017

Conversation

remittor
Copy link

@remittor remittor commented May 24, 2017

openssh-winscp

@remittor remittor force-pushed the v75-enum-root-dir branch 2 times, most recently from 09138e2 to 02fc287 Compare May 24, 2017 10:25
Enum logical drives using GetLogicalDriveStringsW.

/* Detect root dir */
if (path && strcmp(path, "/") == 0) {
buf->st_mode = _S_IFDIR | _S_IREAD | 0xFF;

Choose a reason for hiding this comment

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

0xff is not required here?

Choose a reason for hiding this comment

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

I think this can also be removed..

/* Detect root dir */
if (path && strcmp(path, "/") == 0) {
buf->st_mode = _S_IFDIR | _S_IREAD | 0xFF;
buf->st_dev = USHRT_MAX; // rootdir flag

Choose a reason for hiding this comment

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

Use ATTR_ROOTDIR to be consistent..

Copy link
Author

Choose a reason for hiding this comment

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

ATTR_ROOTDIR is 32 bit
buf->st_dev is 16 bit

Choose a reason for hiding this comment

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

what is the need for setting this here as we are not using this any where else?

Copy link
Author

Choose a reason for hiding this comment

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

You can remove this line, if it so cuts your eyes.

Choose a reason for hiding this comment

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

We should remove the unnecessary code so its easy to maintain the long run..

continue;

x = GetDiskFreeSpaceExW(p, NULL, &totalNumberOfBytes, NULL);
if (!x || totalNumberOfBytes.QuadPart == 0)

Choose a reason for hiding this comment

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

I think, we can remove "totalNumberOfBytes.QuadPart == 0"

Copy link
Author

Choose a reason for hiding this comment

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

Var totalNumberOfBytes.QuadPart equ 0 when CD-drive have not CD-disk.

memset(buf, 0, sizeof(struct _stat64));

/* Detect root dir */
if (path && strcmp(path, "/") == 0) {
Copy link

@bagajjal bagajjal May 24, 2017

Choose a reason for hiding this comment

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

from winscp (or) any other third party software.. if we receive "/" then it is treated as the root dir....

But I am not sure if all the thirdparty softwares follow the same notion.. not sure if some of them can use \ instead of "/", is it possible.. is there a standard notion?

image

Copy link
Author

Choose a reason for hiding this comment

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

I tested this PR in WinSCP and Bitvise SFTP-clients.

@manojampalam manojampalam merged commit 1d53705 into PowerShell:latestw_all May 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants