Include version in public header

[Flamefire]

It would be helpful for consumers to know the version of the library during compilation.

So I propose to define at least LIBSAMPLERATE_VERSION in samplerate.h.

A good way done by other projects is to have _VERSION a string, and _VERSION_[MAJOR|MINOR|PATCH] as 3 additional defines.

Examples: https://github.com/opencv/opencv/blob/master/modules/core/include/opencv2/core/version.hpp or https://github.com/esp8266/Arduino/blob/master/tools/sdk/include/version.h

Boost uses a single number: https://www.boost.org/doc/libs/1_70_0/boost/version.hpp

Example uses:

#ifndef LIBSAMPLERATE_VERSION_MAJOR
  code for pre 0.1.10
#elif LIBSAMPLERATE_VERSION_MAJOR >=1
  code for 1.x
#elif ...

[erikd]

The API for libsamplerate has not changed since its first release in 2002. In 2004 a couple of new functions were added to the API.

You are proposing a solution to a problem that does not yet exist. When (and if) the API changes a version number can be added.

[Flamefire]

In 2004 a couple of new functions were added to the API.

Well then there should have been a version added in 2004.

You are proposing a solution to a problem that does not yet exist.

Older versions may have bugs specific users run into but they may still want to support them by providing work-arounds. Providing the version in the header is very low effort and can help a lot under specific circumstances.

Additionally a version in the header helps to write a CMake FindModule. If this is not chosen I'd propose to add a CMakeConfig additionally to the PkgCfg which helps for non-unix systems where pkgcfg is not commonly used.

[erikd]

Older versions may have bugs specific user

Any version other than the latest release us unsupported. When a new release comes out, I support the very last release for up to 6 months.

Additionally a version in the header helps to write a CMake FindModule.

People have contributed CMake build systems for other projects I maintain without doing this. You should figure out how to add CMake support without doing this.

[Flamefire]

Any version other than the latest release us unsupported.

I think you got it the wrong way round. I was talking about users supporting to use an older release of libsamplerate in their project, e.g. because that is the version bundled with package managers and therefore most commonly used.

How is a user supposed to find out which version he is using? Only current way is through pkgconfig which requires a configure step and setting a version define for each project. And pkgconfig is not even supported on all platforms

I don't understand the hard feelings against this. It is a commonly used approach by many libraries, very easy to implement (1-4 lines) especially easy to test (if not using a configure step to generate that version on build which would remove the need to write a test) and may help users.

[erikd]

I think you got it the wrong way round. I was talking about users supporting to use an older release of libsamplerate in their project, e.g. because that is the version bundled with package managers and therefore most commonly used.

It is still completely un-necessary because the libsamplerate API (and in fact application binary interface, with the exception of some added functions) has remained un-changed since its first release.

How is a user supposed to find out which version he is using?

If the API has remained un-changed for the last several releases why does this even matter?

I don't understand the hard feelings against this.

An unnecessary change is unnecessary.

[Flamefire]

If the API has remained un-changed for the last several releases why does this even matter?

• To find out if he is using the latest version
• To find out if he needs to work-around a known bug in an older version than the latest
• Do that at configure and/or compile inside build scripts run at other peoples computers

[erikd]

To find out if he is using the latest version

The package manager can tell them which version but can't tell them if its the latest (they should go to the website for that). Also remember that the header and the library binary may be out of sync.

To find out if he needs to work-around a known bug in an older version than the latest.

Which bugs?

Do that at configure and/or compile inside build scripts run at other peoples computers

If there has only ever been one API for this library, why would configure scripts need to know anything more than presence/absence?

[Flamefire]

Also remember that the header and the library binary may be out of sync.

True. But if they are installed together this is unlikely.

Which bugs?

Example: #11 with CVE-2017-7697

If there has only ever been one API for this library, why would configure scripts need to know anything more than presence/absence?

To for example require a specific version. E.g one without CVE-2017-7697. The configure runs on somebody elses computer. You (usually) can't just tell them: Check your package manager (if you even have one) for versions of library X, Y and Z to be above a, b and c.
To automate that you either do that at configure time (Search for libX version >=a) or at compile time (#if LIBX_VERSION < a #error "Unsupported version")

[erikd]

True. But if they are installed together this is unlikely.

Linux distros usually get it right. The problem is when people mix locally installed and package installed libraries and headers.

Example: #11 with CVE-2017-7697

Version 0.1.9 was released in 2016 which already contained a fix for that. Unfortunately the history page was not updated to include this release, and the git history actually calls it 1.0.9 which shows I rushed this release out the door as soon as I found the bug. Notice that I found the bug before the CVE was published. Also the download page only lists 0.1.9, so they are encouraged to get the latest version.

If anyone does have a machine with version 0.1.8 of libsamplerate, they probably have dozens of other libraries and applications with far worse CVEs than that one.

I remain unconvinced that adding a version number to the header file adds any real benefit. However, I could pretty easily be convinced to add a function to the library of type:

const char * src_version (void) ;

because that cannot go wrong the way a version number in a header file can.

[Flamefire]

The problem is when people mix locally installed and package installed libraries and headers.

Well if they somehow manage to get /opt/src/include/samplerate.h together with /usr/local/lib/libsamplerate.a then whatever. Can't account for everything.

Version 0.1.9 was released in 2016

This is just an example. What else was done after 0.1.9 which gets into 0.1.10 which users might be interested in? In #67 a user already requested to NOT have a configure step which would be able to find out the version from a package manager. For those cases (assuming an installed libsamplerate but no configure step in user projects) the header would be the only way to assert a version at compile time.

If you really want the extra safety why not use both? A user could then add something like if(LIBSAMPLERATE_VERSION != src_version()) throw error in their startup

BTW: I'd rather use a mangled numeric number (see boost) to be able to do: if(src_version() >= 109) or similar (e.g. if(src_version() >= SRC_MAKE_VERSION(0,1,9)).

Edit: How would the user be supposed to know if src_version() is available without a LIBSAMPLERATE_VERSION define to check against?

[Flamefire]

It is still completely un-necessary because the libsamplerate API (and in fact application binary interface, with the exception of some added functions) has remained un-changed since its first release.

Unfortunately I just ran into an issue which shows that this is not true: src_clone was added after 0.1.9 causing builds against system libsamplerate to fail (wrote the code against the git version): 945b993