All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kris Van Hees <kris.van.hees@oracle.com>
To: Kris Van Hees <kris.van.hees@oracle.com>
Cc: eugene.loh@oracle.com, dtrace@lists.linux.dev,
	dtrace-devel@oss.oracle.com
Subject: Re: [DTrace-devel] [PATCH 2/4] Eliminate DT_VERS_LATEST
Date: Thu, 27 Feb 2025 22:19:23 -0500	[thread overview]
Message-ID: <Z8ErOwkTNsVpjI+M@oracle.com> (raw)
In-Reply-To: <Z8CZeWljSdAozsZH@oracle.com>

On Thu, Feb 27, 2025 at 11:57:29AM -0500, Kris Van Hees wrote:
> There is some overlap with the patch that follows this one, and that got me
> thinking a bit more...
> 
> I think that we should rework all this a little bit, and have dt_version.h be
> the sole source of version number data.  We can put macros in there that
> provide all the version information (strings and codes), and use that to
> populate everything else, I think.  That way we reduce the places where changes
> need to be made (somewhat).
> 
> Let me muse on that a little and I'll follow-up with another email soon with
> a example of what I am envisioing.

See the patch I just posted: 

    [PATCH] Refactor the versioning handling system

I think it might accomplish the same thing, i.e. consolidating the places where
changes are needed for new versions?  I was going to just do some prototype of
a possible solution but that one didn't work out (it needed to expose too much
of the version handling to other code), and then I came up with this idea.

Open to suggestions/feedback/...

> On Thu, Feb 27, 2025 at 11:40:45AM -0500, Kris Van Hees via DTrace-devel wrote:
> > On Sat, Feb 08, 2025 at 02:06:20PM -0500, eugene.loh@oracle.com wrote:
> > > From: Eugene Loh <eugene.loh@oracle.com>
> > > 
> > > Updating the DTrace version number requires too many distinct
> > > changes.  Eliminate DT_VERS_LATEST, since it can be determined
> > > on the fly.
> > > 
> > > Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
> > > ---
> > >  libdtrace/dt_open.c    |  3 ++-
> > >  libdtrace/dt_version.h | 14 +++++---------
> > >  2 files changed, 7 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/libdtrace/dt_open.c b/libdtrace/dt_open.c
> > > index a02058871..b4d160359 100644
> > > --- a/libdtrace/dt_open.c
> > > +++ b/libdtrace/dt_open.c
> > > @@ -721,7 +721,8 @@ dt_vopen(int version, int flags, int *errp,
> > >  	dtp->dt_proc_fd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK);
> > >  	if (dt_aggregate_init(dtp) == -1)
> > >  		return set_open_errno(dtp, errp, dtrace_errno(dtp));
> > > -	dtp->dt_vmax = DT_VERS_LATEST;
> > > +	for (i = 0; _dtrace_versions[i] != 0; i++)
> > > +		dtp->dt_vmax = _dtrace_versions[i];
> > 
> > dtp->dt_vmax = _dtrace_versions[ARRAY_SIZE(_dtrace_versions) - 2];
> > 
> > (-2 to account for the 0 sentinel value)
> > 
> > But this will only be accurate if you also add 2.0.1 to the _dtrace_versions
> > array, and then you should probably add 2.0.2 to it also for accuracy.
> > 
> > >  	dtp->dt_cpp_path = strdup(_dtrace_defcpp);
> > >  	dtp->dt_cpp_argv = malloc(sizeof(char *));
> > >  	dtp->dt_cpp_argc = 1;
> > > diff --git a/libdtrace/dt_version.h b/libdtrace/dt_version.h
> > > index 3fd1b3d1e..bef3243e9 100644
> > > --- a/libdtrace/dt_version.h
> > > +++ b/libdtrace/dt_version.h
> > > @@ -38,18 +38,15 @@ extern "C" {
> > >   *
> > >   * These #defines are used in identifier tables to fill in the version fields
> > >   * associated with each identifier.  The DT_VERS_* macros declare the encoded
> > > - * integer values of all versions used so far.  DT_VERS_LATEST must correspond
> > > - * to the latest version value among all versions exported by the D compiler.
> > > - * DT_VERS_STRING must be an ASCII string that contains DT_VERS_LATEST within
> > > - * it along with any suffixes (e.g. Beta).
> > > + * integer values of all versions used so far.  DT_VERS_STRING must be an ASCII
> > > + * string that contains the latest version within it along with any suffixes
> > > + * (e.g. Beta).  You must update DT_VERS_STRING when adding a new version,
> > > + * and then add the new version to the _dtrace_versions[] array declared in
> > > + * dt_open.c.
> > >   *
> > >   * Refer to the Solaris Dynamic Tracing Guide Versioning chapter for an
> > >   * explanation of these DTrace features and their values.
> > >   *
> > > - * You must update DT_VERS_LATEST and DT_VERS_STRING when adding a new version,
> > > - * and then add the new version to the _dtrace_versions[] array declared in
> > > - * dt_open.c..
> > > - *
> > >   * NOTE: Although the DTrace versioning scheme supports the labeling and
> > >   *       introduction of incompatible changes (e.g. dropping an interface in a
> > >   *       major release), the libdtrace code does not currently support this.
> > > @@ -85,7 +82,6 @@ extern "C" {
> > >  #define	DT_VERS_2_0	DT_VERSION_NUMBER(2, 0, 0)
> > >  #define	DT_VERS_2_0_1	DT_VERSION_NUMBER(2, 0, 1)
> > >  
> > > -#define	DT_VERS_LATEST	DT_VERS_2_0_1
> > >  #define	DT_VERS_STRING	"Oracle D 2.0"
> > 
> > You should add 2.0.2 and update the DT_VERS_STRING also.
> > 
> > >  
> > >  #ifdef  __cplusplus
> > > -- 
> > > 2.43.5
> > > 
> > 
> > _______________________________________________
> > DTrace-devel mailing list
> > DTrace-devel@oss.oracle.com
> > https://oss.oracle.com/mailman/listinfo/dtrace-devel

  reply	other threads:[~2025-02-28  3:19 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-08 19:06 [PATCH 1/4] Rename _DTRACE_VERSION eugene.loh
2025-02-08 19:06 ` [PATCH 2/4] Eliminate DT_VERS_LATEST eugene.loh
2025-02-27 16:40   ` Kris Van Hees
2025-02-27 16:57     ` [DTrace-devel] " Kris Van Hees
2025-02-28  3:19       ` Kris Van Hees [this message]
2025-02-08 19:06 ` [PATCH 3/4] Sync up the version numbers eugene.loh
2025-02-08 19:06 ` [PATCH 4/4] test: Add test for predefined preprocessor definitions eugene.loh
2025-03-18 19:18   ` Kris Van Hees
2025-03-18 20:35     ` Eugene Loh
2025-03-18 20:42       ` Kris Van Hees
2025-02-27 16:27 ` [PATCH 1/4] Rename _DTRACE_VERSION Kris Van Hees

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z8ErOwkTNsVpjI+M@oracle.com \
    --to=kris.van.hees@oracle.com \
    --cc=dtrace-devel@oss.oracle.com \
    --cc=dtrace@lists.linux.dev \
    --cc=eugene.loh@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.