ronantakizawa/codereview-bench huggingface dataset:
Repository: dotnet/runtime
File: src/mono/mono/metadata/threads.c (C)

Code before:
{
	if (mono_thread_start_cb) {
		mono_thread_start_cb (tid, stack_start, func);
	}
}

/**
 * mono_threads_set_default_stacksize:
 */
void
mono_threads_set_default_stacksize (guint32 stacksize)
{
	default_stacksize = stacksize;
}

/**
 * mono_threads_get_default_stacksize:
 */
guint32
mono_threads_get_default_stacksize (void)
{
	if (default_stacksize == ~0)
	{
		unsigned long stacksize = 0;

		const char *value = getenv ("DOTNET_Thread_DefaultStackSize");
		if (value) {
			errno = 0;
			stacksize = strtoul (value, NULL, 16);
			if (errno != 0 || stacksize >= UINT_MAX)
				stacksize = 0;
		}

		default_stacksize = (guint32)stacksize;
	}

	return default_stacksize;
}

/*
 * mono_thread_create_internal:
 *
 *   ARG should not be a GC reference.
 */
MonoInternalThread*
mono_thread_create_internal (MonoThreadStart func, gpointer arg, MonoThreadCreateFlags flags, MonoError *error)
{
	MonoThread *thread;
	MonoInternalThread *internal;
	gboolean res;


Review comment (suggestion):
this should use g_getenv

```suggestion
		const char *value = g_getenv ("DOTNET_Thread_DefaultStackSize");
```

Code after:
{
	if (mono_thread_start_cb) {
		mono_thread_start_cb (tid, stack_start, func);
	}
}

/**
 * mono_threads_set_default_stacksize:
 */
void
mono_threads_set_default_stacksize (guint32 stacksize)
{
	default_stacksize = stacksize;
}

/**
 * mono_threads_get_default_stacksize:
 */
guint32
mono_threads_get_default_stacksize (void)
{
	if (default_stacksize == ~0)
	{
		unsigned long stacksize = 0;

		const char *value = g_getenv ("DOTNET_Thread_DefaultStackSize");
		if (value) {
			errno = 0;
			stacksize = strtoul (value, NULL, 16);
			if (errno != 0 || stacksize >= UINT_MAX)
				stacksize = 0;
		}

		default_stacksize = (guint32)stacksize;
	}

	return default_stacksize;
}

/*
 * mono_thread_create_internal:
 *
 *   ARG should not be a GC reference.
 */
MonoInternalThread*
mono_thread_create_internal (MonoThreadStart func, gpointer arg, MonoThreadCreateFlags flags, MonoError *error)
{
	MonoThread *thread;
	MonoInternalThread *internal;
	gboolean res;
Repository: dotnet/runtime
File: src/mono/mono/utils/mono-threads-wasm.c (C)

Code before:
	g_assert (*stsize != (size_t)-1);
#elif defined(HOST_WASI) && !defined(DISABLE_THREADS)
	// TODO: this will need changes for WASI multithreading as the stack will be allocated per thread at different addresses
	g_assert_not_reached ();
#else
	*staddr = (guint8*)get_wasm_stack_low ();
	*stsize = wasm_get_stack_size ();
#endif

	g_assert ((guint8*)&tmp > *staddr);
	g_assert ((guint8*)&tmp < (guint8*)*staddr + *stsize);
}

gboolean
mono_thread_platform_create_thread (MonoThreadStart thread_fn, gpointer thread_data, gsize* const stack_size, MonoNativeThreadId *tid)
{
#ifdef __EMSCRIPTEN_PTHREADS__
	pthread_attr_t attr;
	pthread_t thread;
	gint res;

	res = pthread_attr_init (&attr);
	if (res != 0)
		g_error ("%s: pthread_attr_init failed, error: \"%s\" (%d)", __func__, g_strerror (res), res);

	/* Actually start the thread */
	res = pthread_create (&thread, &attr, (gpointer (*)(gpointer)) thread_fn, thread_data);
	if (res) {
		res = pthread_attr_destroy (&attr);
		if (res != 0)
			g_error ("%s: pthread_attr_destroy failed, error: \"%s\" (%d)", __func__, g_strerror (res), res);

		return FALSE;
	}

	if (tid)
		*tid = thread;

#if 0
	if (stack_size) {
		res = pthread_attr_getstacksize (&attr, stack_size);
		if (res != 0)
			g_error ("%s: pthread_attr_getstacksize failed, error: \"%s\" (%d)", __func__, g_strerror (res), res);
	}
#endif

	res = pthread_attr_destroy (&attr);
	if (res != 0)
		g_error ("%s: pthread_attr_destroy failed, error: \"%s\" (%d)", __func__, g_strerror (res), res);

	return TRUE;

Review comment (suggestion):
this is disabled anyway but I'd prefer keeping and fixing the disabled code so that we don't diverge with mono-threads-posix.c

Code after:
	g_assert (*stsize != (size_t)-1);
#elif defined(HOST_WASI) && !defined(DISABLE_THREADS)
	// TODO: this will need changes for WASI multithreading as the stack will be allocated per thread at different addresses
	g_assert_not_reached ();
#else
	*staddr = (guint8*)get_wasm_stack_low ();
	*stsize = wasm_get_stack_size ();
#endif

	g_assert ((guint8*)&tmp > *staddr);
	g_assert ((guint8*)&tmp < (guint8*)*staddr + *stsize);
}

gboolean
mono_thread_platform_create_thread (MonoThreadStart thread_fn, gpointer thread_data, gsize* const stack_size, MonoNativeThreadId *tid)
{
#ifdef __EMSCRIPTEN_PTHREADS__
	pthread_attr_t attr;
	pthread_t thread;
	gint res;

	res = pthread_attr_init (&attr);
	if (res != 0)
		g_error ("%s: pthread_attr_init failed, error: \"%s\" (%d)", __func__, g_strerror (res), res);

#if 0
	if (stack_size)
		set_stack_size = *stack_size;
	else
		set_stack_size = 0;

#ifdef HAVE_PTHREAD_ATTR_SETSTACKSIZE
	if (set_stack_size == 0) {
#if HAVE_VALGRIND_MEMCHECK_H
		if (RUNNING_ON_VALGRIND)
			set_stack_size = 1 << 20;
		else
#endif
		{
			set_stack_size = MONO_DEFAULT_STACKSIZE;
		}
	}

#ifdef PTHREAD_STACK_MIN
	if (set_stack_size < PTHREAD_STACK_MIN)
		set_stack_size = PTHREAD_STACK_MIN;
#endif

	res = pthread_attr_setstacksize (&attr, set_stack_size);
	if (res != 0)
		g_error ("%s: pthread_attr_setstacksize failed, error: \"%s\" (%d)", __func__, g_strerror (res), res);
Repository: dotnet/runtime
File: src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeFeature.cs (C#)

Code before:
        /// is not supported or is disabled, such as single-threaded browser environments and WASI.
        /// </summary>
        [UnsupportedOSPlatformGuard("browser")]
        [UnsupportedOSPlatformGuard("wasi")]
        [FeatureSwitchDefinition("System.Runtime.CompilerServices.RuntimeFeature.IsMultithreadingSupported")]
        public static bool IsMultithreadingSupported
#if FEATURE_SINGLE_THREADED
            => false;
#else
            => true;
#endif

#if FEATURE_SINGLE_THREADED
        [DoesNotReturn]
        internal static void ThrowIfMultithreadingIsNotSupported()
        {
            throw new PlatformNotSupportedException();
        }
#else // FEATURE_SINGLE_THREADED
        internal static void ThrowIfMultithreadingIsNotSupported()
        {
#if FEATURE_WASM_MANAGED_THREADS
            System.Threading.Thread.AssureBlockingPossible();
#endif // FEATURE_WASM_MANAGED_THREADS
        }
#endif // FEATURE_SINGLE_THREADED
    }
}


Review comment (suggestion):
```suggestion
#elif FEATURE_WASM_MANAGED_THREADS
        internal static void ThrowIfMultithreadingIsNotSupported()
        {
            System.Threading.Thread.AssureBlockingPossible();
        }
#else
        partial static void ThrowIfMultithreadingIsNotSupported();
endif
```
This should allow the calls to evaporate completely in non-wasm builds.

Code after:
        /// is not supported or is disabled, such as single-threaded browser environments and WASI.
        /// </summary>
        [UnsupportedOSPlatformGuard("browser")]
        [UnsupportedOSPlatformGuard("wasi")]
        [FeatureSwitchDefinition("System.Runtime.CompilerServices.RuntimeFeature.IsMultithreadingSupported")]
        public static bool IsMultithreadingSupported
#if FEATURE_SINGLE_THREADED
            => false;
#else
            => true;
#endif

#if FEATURE_SINGLE_THREADED
        [DoesNotReturn]
        internal static void ThrowIfMultithreadingIsNotSupported()
        {
            throw new PlatformNotSupportedException();
        }
#elif FEATURE_WASM_MANAGED_THREADS
        internal static void ThrowIfMultithreadingIsNotSupported()
        {
            System.Threading.Thread.AssureBlockingPossible();
        }
#else
        [Conditional("unnecessary")]
        internal static void ThrowIfMultithreadingIsNotSupported()
        {
        }
#endif
    }
}
Repository: dotnet/runtime
File: src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeFeature.cs (C#)

Code before:
        /// returns <see langword="false"/> on platforms or configurations where multithreading
        /// is not supported or is disabled, such as single-threaded browser environments and WASI.
        /// </summary>
        [UnsupportedOSPlatformGuard("browser")]
        [UnsupportedOSPlatformGuard("wasi")]
        [FeatureSwitchDefinition("System.Runtime.CompilerServices.RuntimeFeature.IsMultithreadingSupported")]
        public static bool IsMultithreadingSupported
#if FEATURE_SINGLE_THREADED
            => false;
#else
            => true;
#endif

#if FEATURE_SINGLE_THREADED
        [DoesNotReturn]
        internal static void ThrowIfMultithreadingIsNotSupported()
        {
            throw new PlatformNotSupportedException();
        }
#elif FEATURE_WASM_MANAGED_THREADS
        internal static void ThrowIfMultithreadingIsNotSupported()
        {
            System.Threading.Thread.AssureBlockingPossible();
        }
#else
        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        internal static void ThrowIfMultithreadingIsNotSupported()
        {
        }
#endif
    }
}


Review comment (suggestion):
```suggestion
        [Conditional("unnecessary")]
```
This is better - it will strip the method calls

Code after:
        /// returns <see langword="false"/> on platforms or configurations where multithreading
        /// is not supported or is disabled, such as single-threaded browser environments and WASI.
        /// </summary>
        [UnsupportedOSPlatformGuard("browser")]
        [UnsupportedOSPlatformGuard("wasi")]
        [FeatureSwitchDefinition("System.Runtime.CompilerServices.RuntimeFeature.IsMultithreadingSupported")]
        public static bool IsMultithreadingSupported
#if FEATURE_SINGLE_THREADED
            => false;
#else
            => true;
#endif

#if FEATURE_SINGLE_THREADED
        [DoesNotReturn]
        internal static void ThrowIfMultithreadingIsNotSupported()
        {
            throw new PlatformNotSupportedException();
        }
#elif FEATURE_WASM_MANAGED_THREADS
        internal static void ThrowIfMultithreadingIsNotSupported()
        {
            System.Threading.Thread.AssureBlockingPossible();
        }
#else
        [Conditional("unnecessary")]
        internal static void ThrowIfMultithreadingIsNotSupported()
        {
        }
#endif
    }
}
Repository: dotnet/runtime
File: src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeFeature.cs (C#)

Code before:
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.Versioning;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;

namespace System.Runtime.CompilerServices
{
    public static partial class RuntimeFeature
    {
        /// <summary>
        /// Name of the Portable PDB feature.
        /// </summary>
        public const string PortablePdb = nameof(PortablePdb);

        /// <summary>
        /// Indicates that this version of runtime supports default interface method implementations.
        /// </summary>
        public const string DefaultImplementationsOfInterfaces = nameof(DefaultImplementationsOfInterfaces);

        /// <summary>
        /// Indicates that this version of runtime supports the Unmanaged calling convention value.
        /// </summary>
        public const string UnmanagedSignatureCallingConvention = nameof(UnmanagedSignatureCallingConvention);

        /// <summary>
        /// Indicates that this version of runtime supports covariant returns in overrides of methods declared in classes.
        /// </summary>
        public const string CovariantReturnsOfClasses = nameof(CovariantReturnsOfClasses);


Review comment (suggestion):
```suggestion
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
```

Code after:
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.Versioning;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;

namespace System.Runtime.CompilerServices
{
    public static partial class RuntimeFeature
    {
        /// <summary>
        /// Name of the Portable PDB feature.
        /// </summary>
        public const string PortablePdb = nameof(PortablePdb);

        /// <summary>
        /// Indicates that this version of runtime supports default interface method implementations.
        /// </summary>
        public const string DefaultImplementationsOfInterfaces = nameof(DefaultImplementationsOfInterfaces);

        /// <summary>
        /// Indicates that this version of runtime supports the Unmanaged calling convention value.
        /// </summary>
        public const string UnmanagedSignatureCallingConvention = nameof(UnmanagedSignatureCallingConvention);

        /// <summary>
        /// Indicates that this version of runtime supports covariant returns in overrides of methods declared in classes.
        /// </summary>
        public const string CovariantReturnsOfClasses = nameof(CovariantReturnsOfClasses);
Repository: dotnet/runtime
File: src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/web-socket.ts (TypeScript)

Code before:
}

type Message = {
    type: number, // WebSocketMessageType
    data: Uint8Array,
    offset: number
}

function resolvedPromise(): Promise<void> | null {
    // signal that we are finished synchronously
    // this is optimization, which doesn't allocate and doesn't require to marshal resolve() call to C# side.
    return null;
}

function rejectedPromise(message: string): Promise<any> | null {
    const resolved = Promise.reject(new Error(message));
    return wrapAsCancelable<void>(resolved);
}

function verifyEnvironment() {
    if (ENVIRONMENT_IS_SHELL) {
        throw new Error("WebSockets are not supported in shell JS engine.");
    }
    if (typeof globalThis.WebSocket !== "function") {
        const message = ENVIRONMENT_IS_NODE
            ? "Please install `ws` npm package to enable networking support. See also https://learn.microsoft.com/en-us/aspnet/core/blazor/supported-platforms"
            : "This browser doesn't support WebSocket API. Please use a modern browser. See also https://learn.microsoft.com/en-us/aspnet/core/blazor/supported-platforms";
        throw new Error(message);
    }
}


Review comment (suggestion):
```suggestion
            ? "Please install `ws` npm package to enable networking support."
```

Code after:
}

type Message = {
    type: number, // WebSocketMessageType
    data: Uint8Array,
    offset: number
}

function resolvedPromise(): Promise<void> | null {
    // signal that we are finished synchronously
    // this is optimization, which doesn't allocate and doesn't require to marshal resolve() call to C# side.
    return null;
}

function rejectedPromise(message: string): Promise<any> | null {
    const resolved = Promise.reject(new Error(message));
    return wrapAsCancelable<void>(resolved);
}

function verifyEnvironment() {
    if (ENVIRONMENT_IS_SHELL) {
        throw new Error("WebSockets are not supported in shell JS engine.");
    }
    if (typeof globalThis.WebSocket !== "function") {
        const message = ENVIRONMENT_IS_NODE
            ? "Please install `ws` npm package to enable networking support."
            : "This browser doesn't support WebSocket API. Please use a modern browser. See also https://learn.microsoft.com/aspnet/core/blazor/supported-platforms";
        throw new Error(message);
    }
}
Repository: dotnet/runtime
File: src/native/libs/System.Native/pal_io.c (C)

Code before:
#if HAVE_FCHMOD
    int32_t result;
    while ((result = fchmod(ToFileDescriptor(fd), (mode_t)mode)) < 0 && errno == EINTR);
    return result;
#else /* HAVE_FCHMOD */
    (void)fd; // unused
    (void)mode; // unused
    return EINTR;
#endif /* HAVE_FCHMOD */
}

int32_t SystemNative_FSync(intptr_t fd)
{
    int fileDescriptor = ToFileDescriptor(fd);

    int32_t result;
#ifdef TARGET_OSX
    while ((result = fcntl(fileDescriptor, F_FULLFSYNC)) < 0 && errno == EINTR);
    if (result >= 0)
        return result;
    // F_FULLFSYNC is not supported on all file systems and handle types (e.g.,
    // network file systems, read-only handles). Fall back to fsync.
    // For genuine I/O errors (e.g., EIO), fsync will also fail and propagate the error.
#endif
    while ((result = fsync(fileDescriptor)) < 0 && errno == EINTR);
    return result;
}

int32_t SystemNative_FLock(intptr_t fd, int32_t operation)
{
    int32_t result;
#if !defined(TARGET_WASI)
    while ((result = flock(ToFileDescriptor(fd), operation)) < 0 && errno == EINTR);
#else /* TARGET_WASI */
    result = EINTR;
#endif /* TARGET_WASI */
    return result;
}

int32_t SystemNative_ChDir(const char* path)
{
    int32_t result;
    while ((result = chdir(path)) < 0 && errno == EINTR);
    return result;
}

int32_t SystemNative_Access(const char* path, int32_t mode)
{
    return access(path, mode);
}


Review comment (bug):
```suggestion
    if (result >= 0)
    {
        return result;
    }

    // F_FULLFSYNC is not supported on all file systems and handle types (e.g.,
    // network file systems, read-only handles). Fall back to fsync.
    // For genuine I/O errors (e.g., EIO), fsync will also fail and propagate the error.
```

Code after:
#if HAVE_FCHMOD
    int32_t result;
    while ((result = fchmod(ToFileDescriptor(fd), (mode_t)mode)) < 0 && errno == EINTR);
    return result;
#else /* HAVE_FCHMOD */
    (void)fd; // unused
    (void)mode; // unused
    return EINTR;
#endif /* HAVE_FCHMOD */
}

int32_t SystemNative_FSync(intptr_t fd)
{
    int fileDescriptor = ToFileDescriptor(fd);

    int32_t result;
#ifdef TARGET_OSX
    while ((result = fcntl(fileDescriptor, F_FULLFSYNC)) < 0 && errno == EINTR);
    if (result >= 0)
    {
        return result;
    }

    // F_FULLFSYNC is not supported on all file systems and handle types (e.g.,
    // network file systems, read-only handles). Fall back to fsync.
    // For genuine I/O errors (e.g., EIO), fsync will also fail and propagate the error.
#endif
    while ((result = fsync(fileDescriptor)) < 0 && errno == EINTR);
    return result;
}

int32_t SystemNative_FLock(intptr_t fd, int32_t operation)
{
    int32_t result;
#if !defined(TARGET_WASI)
    while ((result = flock(ToFileDescriptor(fd), operation)) < 0 && errno == EINTR);
#else /* TARGET_WASI */
    result = EINTR;
#endif /* TARGET_WASI */
    return result;
}

int32_t SystemNative_ChDir(const char* path)
{
    int32_t result;
    while ((result = chdir(path)) < 0 && errno == EINTR);
    return result;
}

int32_t SystemNative_Access(const char* path, int32_t mode)
{
Repository: dotnet/runtime
File: src/native/libs/System.Security.Cryptography.Native/pal_x509.c (C)

Code before:
int32_t CryptoNative_EncodeX509SubjectPublicKeyInfo(X509* x509, uint8_t* buf)
{
    ERR_clear_error();

    if (!x509)
    {
        return 0;
    }

    // X509_get_X509_PUBKEY returns an interior pointer, so should not be freed
    return i2d_X509_PUBKEY(X509_get_X509_PUBKEY(x509), &buf);
}

X509* CryptoNative_X509UpRef(X509* x509)
{
    if (x509 != NULL)
    {
        // Just a field mutator, no error queue interactions apply.
        X509_up_ref(x509);
    }

    return x509;
}

// On some platforms (like illumos), d_name is declared as char[1], so sizeof(d_name) doesn't give
// the actual maximum length. Use NAME_MAX or MAXNAMLEN instead.
#ifdef NAME_MAX
static size_t maxNameLen = NAME_MAX;
#else
static size_t maxNameLen = 255; // Reasonable default
#endif

static DIR* OpenUserStore(const char* storePath, char** pathTmp, size_t* pathTmpSize, char** nextFileWrite)
{
    DIR* trustDir = opendir(storePath);

    if (trustDir == NULL)
    {
        *pathTmp = NULL;
        *nextFileWrite = NULL;
        return NULL;
    }

    size_t storePathLen = strlen(storePath);

    // Allocate enough space for the store path, a '/', the maximum filename length, and a null terminator

    size_t allocSize = storePathLen + maxNameLen + 2; // +2 for '/' and '\0'
    char* tmp = (char*)calloc(allocSize, sizeof(char));
    if (!tmp)
    {

Review comment (suggestion):
The comment mentions MAXNAMLEN, but the code doesn't.  AFAICT, MAXNAMLEN is a BSD-only name, but that BSD also defines the POSIX NAME_MAX, so MAXNAMLEN should just be removed from the comment.

Code after:
    // X509_get_X509_PUBKEY returns an interior pointer, so should not be freed
    return i2d_X509_PUBKEY(X509_get_X509_PUBKEY(x509), NULL);
}

int32_t CryptoNative_EncodeX509SubjectPublicKeyInfo(X509* x509, uint8_t* buf)
{
    ERR_clear_error();

    if (!x509)
    {
        return 0;
    }

    // X509_get_X509_PUBKEY returns an interior pointer, so should not be freed
    return i2d_X509_PUBKEY(X509_get_X509_PUBKEY(x509), &buf);
}

X509* CryptoNative_X509UpRef(X509* x509)
{
    if (x509 != NULL)
    {
        // Just a field mutator, no error queue interactions apply.
        X509_up_ref(x509);
    }

    return x509;
}

static DIR* OpenUserStore(const char* storePath, char** pathTmp, size_t* pathTmpSize, char** nextFileWrite)
{
    DIR* trustDir = opendir(storePath);

    if (trustDir == NULL)
    {
        *pathTmp = NULL;
        *nextFileWrite = NULL;
        return NULL;
    }

    size_t storePathLen = strlen(storePath);

    // d_name is a fixed length char[], not a char*.
    // The traditional declaration (eg. SunOS) is d_name[1]
    // so we can't assume sizeof(ent->d_name) will work,
    // so just use NAME_MAX which is the POSIX way.
    // Leave one byte for '\0' and one for '/'
    size_t allocSize = storePathLen + NAME_MAX + 2;
    char* tmp = (char*)calloc(allocSize, sizeof(char));
    if (!tmp)
    {
        closedir(trustDir);