Re: svn commit: r1915215 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_wc/revert.c svn/notify.c svnbench/notify.c

[Daniel Sahlberg]

Den mån 15 dec. 2025 kl 15:32 skrev Evgeny Kotkov via dev <
dev@subversion.apache.org>:

> Daniel Sahlberg <dsahlb...@apache.org> writes:
>
> >    svn_boolean_t notify_required;
> > +  svn_boolean_t notify_noaccess;
> …
> >    if (!metadata_only)
> >      {
> >        SVN_ERR(revert_wc_data(run_wq,
> > -                             &notify_required,
> > +                             &notify_required, &notify_noaccess,
> >                               db, local_abspath, status, kind,
> >                               reverted_kind, recorded_size,
> recorded_time,
> >                               copied_here, use_commit_times,
> …
> > +  if (notify_func)
> > +    {
> > +      if (notify_required)
> > +        notify_func(notify_baton,
> > +                    svn_wc_create_notify(local_abspath,
> svn_wc_notify_revert,
> > +                                         scratch_pool),
> > +                    scratch_pool);
> > +      else if (notify_noaccess)
> > +        notify_func(notify_baton,
> > +                    svn_wc_create_notify(local_abspath,
> svn_wc_notify_revert_noaccess,
> > +                                         scratch_pool),
> > +                    scratch_pool);
> > +    }
>
> It looks like this may have introduced an undefined behavior for reverts
> with
> metadata_only = TRUE.
>
> If metadata_only is TRUE, the block calling revert_wc_data is skipped,
> meaning
> notify_noaccess is never assigned a value.  However, the variable might
> still
> be read later in the `else if (notify_noaccess)` check.
>

You are right of course, good catch.

How about:

[[[
$ svn diff libsvn_wc/revert.c
Index: libsvn_wc/revert.c
===================================================================
--- libsvn_wc/revert.c  (revision 1930472)
+++ libsvn_wc/revert.c  (working copy)
@@ -310,7 +310,7 @@
   svn_wc__db_status_t status;
   svn_node_kind_t kind;
   svn_boolean_t notify_required;
-  svn_boolean_t notify_noaccess;
+  svn_boolean_t notify_noaccess = FALSE;
   const apr_array_header_t *conflict_files;
   svn_filesize_t recorded_size;
   apr_time_t recorded_time;
]]]

Cheers,
Daniel

>
>