Skip to content
This repository has been archived by the owner on Oct 17, 2023. It is now read-only.

Add support for Windows #91

Merged
merged 6 commits into from
Feb 23, 2018
Merged

Add support for Windows #91

merged 6 commits into from
Feb 23, 2018

Conversation

zavitax
Copy link
Contributor

@zavitax zavitax commented Feb 10, 2018

Provide Windows versions of sys/mman.h functions.
Use open() calls suitable for Windows when building on WIN32

@zavitax zavitax mentioned this pull request Feb 10, 2018
Copy link
Member

@oschwald oschwald left a comment

Choose a reason for hiding this comment

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

Thanks! I am not set up to test it on Windows, but this seems pretty reasonable. I made a couple minor comments.

Longer term, we should probably look into setting up AppVeyor to test this if we want to support Windows.

c/tree.c Outdated
#include <fcntl.h>
#include <errno.h>
#include <inttypes.h>
#include <netdb.h>
#include <stdbool.h>
#include <stdio.h>

#ifdef WIN32
// Source: https://github.com/witwall/mman-win32
Copy link
Member

Choose a reason for hiding this comment

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

Please split out this code into a separate C file with clear licensing information. It appears that it is under the MIT license.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oschwald done

c/tree.c Outdated

static int __map_mman_error(const DWORD err, const int deferr)
{
if (err == 0)
Copy link
Member

Choose a reason for hiding this comment

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

After splitting out the code, please run dev-bin/uncrustify-all.sh on it so that it is formatted similarly to our other code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oschwald win32 code has been split into separate files and passed through dev-bin/uncrustify-all.sh

Copy link
Contributor Author

@zavitax zavitax left a comment

Choose a reason for hiding this comment

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

win32 changes split into separate files and changes passed through dev-bin/uncrustify-all.sh

c/tree.c Outdated

static int __map_mman_error(const DWORD err, const int deferr)
{
if (err == 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.

@oschwald win32 code has been split into separate files and passed through dev-bin/uncrustify-all.sh

c/tree.c Outdated
#include <fcntl.h>
#include <errno.h>
#include <inttypes.h>
#include <netdb.h>
#include <stdbool.h>
#include <stdio.h>

#ifdef WIN32
// Source: https://github.com/witwall/mman-win32
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oschwald done

@oschwald
Copy link
Member

Thanks! This looks great.

@oschwald oschwald merged commit 78cedb6 into maxmind:master Feb 23, 2018
@zavitax
Copy link
Contributor Author

zavitax commented Feb 24, 2018

@oschwald my pleasure!

@zavitax
Copy link
Contributor Author

zavitax commented Mar 8, 2018

@oschwald can you kindly add my name/email to the list of contributors?

@oschwald
Copy link
Member

oschwald commented Mar 8, 2018

The list is generated on release. I just did a release so it should now appear there. Thanks for the reminder!

@zavitax
Copy link
Contributor Author

zavitax commented Mar 8, 2018

Thanks! I'm wondering, why didn't it use my full last name?

@oschwald
Copy link
Member

oschwald commented Mar 8, 2018

It gets the names from the Git history. If you want something different to appear, the .mailmap file in the distro root can be updated.

@zavitax
Copy link
Contributor Author

zavitax commented Mar 8, 2018

I would appreciate if you could update ilya77@gmail.com -> Ilya Melamed

@oschwald
Copy link
Member

oschwald commented Mar 8, 2018

Done in 2488748. Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants