You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Nathan Hartman <ha...@gmail.com> on 2025/12/12 18:58:06 UTC
Re: svn commit: r1930473 - subversion/trunk/subversion/libsvn_subr
On Fri, Dec 12, 2025 at 1:27 PM <ds...@apache.org> wrote:
> Author: dsahlberg
> Date: Fri Dec 12 18:27:04 2025
> New Revision: 1930473
>
> Log:
> Restore the behaviour before r1915214.
>
> * subversion/libsvn_subr/io.c
> (svn_io__is_finfo_read_only): As above
> (svn_io__is_finfo_executable): As above
> (svn_io_is_file_executable): As above
>
> Modified:
> subversion/trunk/subversion/libsvn_subr/io.c
>
> Modified: subversion/trunk/subversion/libsvn_subr/io.c
>
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/io.c Fri Dec 12
> 18:08:09 2025 (r1930472)
> +++ subversion/trunk/subversion/libsvn_subr/io.c Fri Dec 12
> 18:27:04 2025 (r1930473)
> @@ -2531,14 +2531,27 @@ svn_io__is_finfo_read_only(svn_boolean_t
> apr_pool_t *pool)
> {
> #if defined(APR_HAS_USER) && !defined(WIN32) &&!defined(__OS2__)
> - *read_only = (access(file_info->fname, W_OK) != 0);
> - /* svn_io__is_finfo_read_only can be called with a dangling
> - * symlink. access() will check the permission on the missing
> - * target and return -1 and errno = ENOENT. Check for ENOENT
> - * and pretend the file is writeable, otherwise we will get
> - * spurious Reverted messages on the symlink.
> - */
> - if (*read_only && errno == ENOENT) *read_only = FALSE;
> + apr_status_t apr_err;
> + apr_uid_t uid;
> + apr_gid_t gid;
> +
> + *read_only = FALSE;
> +
> + apr_err = apr_uid_current(&uid, &gid, pool);
> +
> + if (apr_err)
> + return svn_error_wrap_apr(apr_err, _("Error getting UID of process"));
> +
> + /* Check write bit for current user. */
> + if (apr_uid_compare(uid, file_info->user) == APR_SUCCESS)
> + *read_only = !(file_info->protection & APR_UWRITE);
> +
> + else if (apr_gid_compare(gid, file_info->group) == APR_SUCCESS)
> + *read_only = !(file_info->protection & APR_GWRITE);
> +
> + else
> + *read_only = !(file_info->protection & APR_WWRITE);
> +
> #else /* WIN32 || __OS2__ || !APR_HAS_USER */
> *read_only = (file_info->protection & APR_FREADONLY);
> #endif
> @@ -2551,7 +2564,33 @@ svn_io__is_finfo_executable(svn_boolean_
> apr_finfo_t *file_info,
> apr_pool_t *pool)
> {
> - return svn_io_is_file_executable(executable, file_info->fname, pool);
> +#if defined(APR_HAS_USER) && !defined(WIN32) &&!defined(__OS2__)
> + apr_status_t apr_err;
> + apr_uid_t uid;
> + apr_gid_t gid;
> +
> + *executable = FALSE;
> +
> + apr_err = apr_uid_current(&uid, &gid, pool);
> +
> + if (apr_err)
> + return svn_error_wrap_apr(apr_err, _("Error getting UID of process"));
> +
> + /* Check executable bit for current user. */
> + if (apr_uid_compare(uid, file_info->user) == APR_SUCCESS)
> + *executable = (file_info->protection & APR_UEXECUTE);
> +
> + else if (apr_gid_compare(gid, file_info->group) == APR_SUCCESS)
> + *executable = (file_info->protection & APR_GEXECUTE);
> +
> + else
> + *executable = (file_info->protection & APR_WEXECUTE);
> +
> +#else /* WIN32 || __OS2__ || !APR_HAS_USER */
> + *executable = FALSE;
> +#endif
> +
> + return SVN_NO_ERROR;
> }
>
> svn_error_t *
> @@ -2560,7 +2599,12 @@ svn_io_is_file_executable(svn_boolean_t
> apr_pool_t *pool)
> {
> #if defined(APR_HAS_USER) && !defined(WIN32) &&!defined(__OS2__)
> - *executable = (access(path, X_OK) == 0);
> + apr_finfo_t file_info;
> +
> + SVN_ERR(svn_io_stat(&file_info, path, APR_FINFO_PROT | APR_FINFO_OWNER,
> + pool));
> + SVN_ERR(svn_io__is_finfo_executable(executable, &file_info, pool));
> +
> #else /* WIN32 || __OS2__ || !APR_HAS_USER */
> *executable = FALSE;
> #endif
>
>
Hi,
May I suggest recording the rationale for this change in the log message?
Thanks,
Nathan
Re: svn commit: r1930473 - subversion/trunk/subversion/libsvn_subr
Posted by Daniel Sahlberg <da...@gmail.com>.
Den fre 12 dec. 2025 kl 18:58 skrev Nathan Hartman <hartman.nathan@gmail.com
>:
> On Fri, Dec 12, 2025 at 1:27 PM <ds...@apache.org> wrote:
>
>> Author: dsahlberg
>> Date: Fri Dec 12 18:27:04 2025
>> New Revision: 1930473
>>
>> Log:
>> Restore the behaviour before r1915214.
>>
>> * subversion/libsvn_subr/io.c
>> (svn_io__is_finfo_read_only): As above
>> (svn_io__is_finfo_executable): As above
>> (svn_io_is_file_executable): As above
>>
>> Modified:
>> subversion/trunk/subversion/libsvn_subr/io.c
>>
>> Modified: subversion/trunk/subversion/libsvn_subr/io.c
>>
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_subr/io.c Fri Dec 12
>> 18:08:09 2025 (r1930472)
>> +++ subversion/trunk/subversion/libsvn_subr/io.c Fri Dec 12
>> 18:27:04 2025 (r1930473)
>> @@ -2531,14 +2531,27 @@ svn_io__is_finfo_read_only(svn_boolean_t
>> apr_pool_t *pool)
>> {
>> #if defined(APR_HAS_USER) && !defined(WIN32) &&!defined(__OS2__)
>> - *read_only = (access(file_info->fname, W_OK) != 0);
>> - /* svn_io__is_finfo_read_only can be called with a dangling
>> - * symlink. access() will check the permission on the missing
>> - * target and return -1 and errno = ENOENT. Check for ENOENT
>> - * and pretend the file is writeable, otherwise we will get
>> - * spurious Reverted messages on the symlink.
>> - */
>> - if (*read_only && errno == ENOENT) *read_only = FALSE;
>> + apr_status_t apr_err;
>> + apr_uid_t uid;
>> + apr_gid_t gid;
>> +
>> + *read_only = FALSE;
>> +
>> + apr_err = apr_uid_current(&uid, &gid, pool);
>> +
>> + if (apr_err)
>> + return svn_error_wrap_apr(apr_err, _("Error getting UID of
>> process"));
>> +
>> + /* Check write bit for current user. */
>> + if (apr_uid_compare(uid, file_info->user) == APR_SUCCESS)
>> + *read_only = !(file_info->protection & APR_UWRITE);
>> +
>> + else if (apr_gid_compare(gid, file_info->group) == APR_SUCCESS)
>> + *read_only = !(file_info->protection & APR_GWRITE);
>> +
>> + else
>> + *read_only = !(file_info->protection & APR_WWRITE);
>> +
>> #else /* WIN32 || __OS2__ || !APR_HAS_USER */
>> *read_only = (file_info->protection & APR_FREADONLY);
>> #endif
>> @@ -2551,7 +2564,33 @@ svn_io__is_finfo_executable(svn_boolean_
>> apr_finfo_t *file_info,
>> apr_pool_t *pool)
>> {
>> - return svn_io_is_file_executable(executable, file_info->fname, pool);
>> +#if defined(APR_HAS_USER) && !defined(WIN32) &&!defined(__OS2__)
>> + apr_status_t apr_err;
>> + apr_uid_t uid;
>> + apr_gid_t gid;
>> +
>> + *executable = FALSE;
>> +
>> + apr_err = apr_uid_current(&uid, &gid, pool);
>> +
>> + if (apr_err)
>> + return svn_error_wrap_apr(apr_err, _("Error getting UID of
>> process"));
>> +
>> + /* Check executable bit for current user. */
>> + if (apr_uid_compare(uid, file_info->user) == APR_SUCCESS)
>> + *executable = (file_info->protection & APR_UEXECUTE);
>> +
>> + else if (apr_gid_compare(gid, file_info->group) == APR_SUCCESS)
>> + *executable = (file_info->protection & APR_GEXECUTE);
>> +
>> + else
>> + *executable = (file_info->protection & APR_WEXECUTE);
>> +
>> +#else /* WIN32 || __OS2__ || !APR_HAS_USER */
>> + *executable = FALSE;
>> +#endif
>> +
>> + return SVN_NO_ERROR;
>> }
>>
>> svn_error_t *
>> @@ -2560,7 +2599,12 @@ svn_io_is_file_executable(svn_boolean_t
>> apr_pool_t *pool)
>> {
>> #if defined(APR_HAS_USER) && !defined(WIN32) &&!defined(__OS2__)
>> - *executable = (access(path, X_OK) == 0);
>> + apr_finfo_t file_info;
>> +
>> + SVN_ERR(svn_io_stat(&file_info, path, APR_FINFO_PROT | APR_FINFO_OWNER,
>> + pool));
>> + SVN_ERR(svn_io__is_finfo_executable(executable, &file_info, pool));
>> +
>> #else /* WIN32 || __OS2__ || !APR_HAS_USER */
>> *executable = FALSE;
>> #endif
>>
>>
> Hi,
>
> May I suggest recording the rationale for this change in the log message?
>
Yes, I should have linked to the mailing list. Log message edited!
Thanks,
Daniel
>
>