Skip to content

Conversation

@ricardocvel
Copy link
Contributor

This PR aims to implement the function:

  • fstatat ()
    for Windows;
    and declare the functions in stat.h:
  • umask ()
  • chmod ()
    (using the header: <corecrt_io.h> and <errno.h>)

Copy link
Contributor

@Coquinho Coquinho left a comment

Choose a reason for hiding this comment

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

Your implementation should be an evil_ not an unposix/, so please move stah{h,c} to evil/evil_stat.{h,c}.

@Coquinho
Copy link
Contributor

how exactly can we test it?

@ricardocvel ricardocvel force-pushed the devs/expertise-ricardo/sys/stat.h branch 3 times, most recently from 5f9f866 to 62d2389 Compare May 19, 2020 17:59
@ricardocvel ricardocvel force-pushed the devs/expertise-ricardo/sys/stat.h branch from 62d2389 to 4235c4f Compare June 5, 2020 19:13
@ricardocvel ricardocvel changed the base branch from devs/felipealmeida/fix-eina-dll-export to devs/expertise/native-windows June 8, 2020 17:44
@ricardocvel ricardocvel force-pushed the devs/expertise-ricardo/sys/stat.h branch from 4235c4f to 598bc10 Compare June 16, 2020 13:06
Copy link
Contributor

@joaoantoniocardoso joaoantoniocardoso left a comment

Choose a reason for hiding this comment

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

I am changing my approval to be clear that some work still needs to be done.

@joaoantoniocardoso joaoantoniocardoso self-requested a review June 16, 2020 15:56
@ricardocvel ricardocvel force-pushed the devs/expertise-ricardo/sys/stat.h branch from 187ed15 to 700df59 Compare June 16, 2020 16:20
Copy link
Contributor

@joaoantoniocardoso joaoantoniocardoso left a comment

Choose a reason for hiding this comment

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

Almost there, just some minor changes for me.

Is there some way to test it?

@JPTIZ JPTIZ added this to the Natively Compile in Windows milestone Jun 16, 2020
@ricardocvel ricardocvel force-pushed the devs/expertise-ricardo/sys/stat.h branch 2 times, most recently from a1ad61f to c506a45 Compare June 17, 2020 12:30
Copy link
Contributor

@Coquinho Coquinho left a comment

Choose a reason for hiding this comment

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

Code seems fine to me, how can I test this?

Copy link
Contributor

@Coquinho Coquinho left a comment

Choose a reason for hiding this comment

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

How can I test this?

And another thing, should these new things be inside some #ifndef CYGWIN(don't remember if it's exactly like that) as this isn't needed when building with it, but only when building windows natively?

Copy link
Contributor

@joaoantoniocardoso joaoantoniocardoso left a comment

Choose a reason for hiding this comment

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

#169 changed EAPI to EVIL_API

ricardocvel and others added 8 commits June 25, 2020 14:57
Update stat.h

Update stat.c

Update stat.h

Update stat.c

Update stat.h

including Windows.h at WIN_LEAN_AND_MEAN.

Co-authored-by: Lucas <Coquinho@users.noreply.github.com>

moving stat. {h, c) for evil / evil stat. {h, c}

deleting items, which were copied to another directory
@ricardocvel ricardocvel force-pushed the devs/expertise-ricardo/sys/stat.h branch from 584be66 to 6f33c48 Compare June 25, 2020 21:06
{
int r_fstatat;

if (pathname[1] == ':' && pathname[2] == '\\' || pathname[2] == '/' )
Copy link
Contributor

Choose a reason for hiding this comment

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

You must check the buffer size before accessing it.

do
{
pathbuf_size += MAX_PATH;
pathbuf = realloc(pathbuf, pathbuf_size * sizeof(char));
Copy link
Contributor

Choose a reason for hiding this comment

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

The memory allocated here is never freed.

}

size_str = strlen(pathbuf) + strlen(pathname);
char *path_complete = malloc(sizeof(char) * size_str + 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

The memory allocated here is never freed.

size_str = strlen(pathbuf) + strlen(pathname);
char *path_complete = malloc(sizeof(char) * size_str + 1);
strcpy(path_complete, pathbuf);
strcat(path_complete, pathname);
Copy link
Contributor

Choose a reason for hiding this comment

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

What if pathbuf doesn't end with a path separator?

if (flags == AT_SYMLINK_NOFOLLOW)
r_fstatat = stat(pathname, statbuf);
else
r_fstatat = stat(pathname, statbuf);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the if both calls are exactly the same?

pathbuf[size_str + 1] = 0;
break;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

What about using _splitpath instead of a manual for loop?

else
{
r_fstatat = stat(path_complete, statbuf);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the if?

Copy link
Contributor

@Coquinho Coquinho left a comment

Choose a reason for hiding this comment

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

Hey guys, what do you think about breaking this in 2 PR? We could have one with the functions, which seems to still have some loose ends, and we can approve the definitions, as it seems there is no discussion there.

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.

6 participants