Finishing up PAL'ing Unix\libc (non-networking and crypto) PInvoke calls #3257
Conversation
|
@dotnet-bot test this please Looks like a Windows test failure around a string comparison...will try to test again. The test suite that failed in this run passes locally |
| { | ||
| internal static int DEFAULT_PC_NAME_MAX = 255; | ||
|
|
||
| internal enum PathConfNames : int |
There was a problem hiding this comment.
This should be PathConfName singular since these aren't flags.
|
Other than the issues called out, LGTM. Good to see us almost through the non-networking/crypto P/Invokes. |
| @@ -344,3 +354,75 @@ int32_t WTermSig(int32_t status) | |||
| { | |||
| return WTERMSIG(status); | |||
| } | |||
|
|
|||
| static | |||
| int32_t ConvertPathConfNameToPlatform(PathConfName& name) | |||
There was a problem hiding this comment.
there is no sense to pass name as a reference
There was a problem hiding this comment.
Agreed :) I deleted the whole function anyway
|
@dotnet-bot test this please |
|
Any further comments folks? @nguerrera @stephentoub |
| if (result != null) | ||
| { | ||
| { |
There was a problem hiding this comment.
nit: trailing whitespace
|
@sokket, by any chance, does this happen to fix https://github.com/dotnet/corefx/issues/2988? |
| @@ -53,6 +54,11 @@ static_assert(PAL_LOG_LOCAL5 == LOG_LOCAL5, ""); | |||
| static_assert(PAL_LOG_LOCAL6 == LOG_LOCAL6, ""); | |||
| static_assert(PAL_LOG_LOCAL7 == LOG_LOCAL7, ""); | |||
|
|
|||
| // Validate that out PriorityWhich values are correct for the platform | |||
| static_assert(static_cast<int>(PriorityWhich::PAL_PRIO_PROCESS) == PRIO_PROCESS, ""); | |||
There was a problem hiding this comment.
I made our enums not have the 'class' designation so that we can get rid of a bunch of casts like this: i.e. enum Foo : T instead of enum class Foo : T. Please fix the new ones up to match.
|
@jasonwilliams200OK I don't think so since this doesn't shim anything in System.Net yet; this could be fixed with my next check-in though :). I'm moving on to shim the System.Net calls that are currently in Master next, so I'll assign that bug to me and make sure that the shim code for System.Net compiles on my FreeBSD box as well |
| { | ||
| // GetPriority uses errno 0 to show succes to make sure we don't have a stale value | ||
| errno = 0; | ||
| return getpriority(static_cast<int>(which), static_cast<id_t>(who)); |
There was a problem hiding this comment.
Note that you also won't need to cast which here after changing from enum class to enum.
As for id_t, are there any range concerns here?
There was a problem hiding this comment.
Nope; it's more because id_t is declared as unsigned but the c# base class declares it as signed, so we need a conversion when sending back to the OS here
|
LGTM for the most part, but I left a few more comments. |
|
Thanks folks! Double checking that the build passes and then will merge |
Finishing up PAL'ing Unix\libc (non-networking and crypto) PInvoke calls
Finishing up PAL'ing Unix\libc (non-networking and crypto) PInvoke calls
/c @nguerrera