RFR: os_linux.cpp parse_os_info gives non descriptive output on current SLES releases

[Coleen Phillimore]

Thank you for finding this problem with SuSE and the diff.  I don't know 
how many different distros were tested with these changes but it's easy 
not to get them all.

Would it make more sense, rather than getting the last line then 
checking for DISTRIB_DESCRIPTION=, checking in the fgets() loop for both 
this and PRETTY_NAME.  Then we could change it for Ubuntu and there's 
less conditionalizing on the name of the release file.

Like:

open webrev at http://cr.openjdk.java.net/~coleenp/suse/webrev

Coleen

On 4/18/16 8:05 AM, Baesken, Matthias wrote:
> Hello ,  the current implementation of the  parse_os_info-function in os_linux.cpp  gets the last line of a Linux-distro related file to
> provide a meaningful OS version string.
>
> However the information provided currently on SuSE Linux (SLES) is not very descriptive, it currently uses /etc/lsb-release and gives :
>
> more /etc/lsb-release
> LSB_VERSION="core-2.0-noarch:core-3.2-noarch:core-4.0-noarch:core-2.0-x86_64:core-3.2-x86_64:core-4.0-x86_64"
>
> So I suggest to use /etc/SuSE-release instead, which gives a good information for
> SLES 9 - 12 in the ***first line*** of  /etc/SuSE-release :
>
> Example SLES11 :
>
> more /etc/SuSE-release
> SUSE Linux Enterprise Server 11 (x86_64)
> VERSION = 11
> PATCHLEVEL = 3
>
> (this is similar to using /etc/redhat-release on Red Hat with the difference that the ***first line*** has the relevant info).
>
>
> Additionally, /etc/os-release needs some special handling as well, because
> the meaningful OS-release description string is not always  the last line of the file but in the line
> containing the information PRETTY_NAME=...
> See also :
>
> https://www.freedesktop.org/software/systemd/man/os-release.html
>
> Example from Ubuntu 14 :
>
> $ more /etc/os-release
>    ...
> PRETTY_NAME="Ubuntu 14.04.3 LTS"
>    ...
>
> It might also be a good idea to place  /etc/os-release  higher in the distro_files list, but I do not have access to
> turbolinux / gentoo to check the situation on these distros.
>
> Regards, Matthias
>
>
>
> Diff :
> --- a/src/os/linux/vm/os_linux.cpp      Fri Apr 15 16:19:15 2016 +0100
> +++ b/src/os/linux/vm/os_linux.cpp      Mon Apr 18 13:54:04 2016 +0200
> @@ -2013,8 +2013,8 @@
> // their own specific XXX-release file as well as a redhat-release file.
> // Because of this the XXX-release file needs to be searched for before the
> // redhat-release file.
> -// Since Red Hat has a lsb-release file that is not very descriptive the
> -// search for redhat-release needs to be before lsb-release.
> +// Since Red Hat and SuSE have an lsb-release file that is not very descriptive the
> +// search for redhat-release / SuSE-release needs to be before lsb-release.
> // Since the lsb-release file is the new standard it needs to be searched
> // before the older style release files.
> // Searching system-release (Red Hat) and os-release (other Linuxes) are a
> @@ -2031,8 +2031,8 @@
>     "/etc/mandrake-release",
>     "/etc/sun-release",
>     "/etc/redhat-release",
> +  "/etc/SuSE-release",
>     "/etc/lsb-release",
> -  "/etc/SuSE-release",
>     "/etc/turbolinux-release",
>     "/etc/gentoo-release",
>     "/etc/ltib-release",
> @@ -2065,11 +2065,36 @@
> static void parse_os_info(char* distro, size_t length, const char* file) {
>     FILE* fp = fopen(file, "r");
>     if (fp != NULL) {
> +    // SuSE-release : first line is interesting
> +    // os-release : PRETTY_NAME= line is interesting
> +    // (might be at different locations in the file)
>       char buf[256];
> -    // get last line of the file.
> -    while (fgets(buf, sizeof(buf), fp)) { }
> +    int lcnt = 0;
> +    bool is_etc_suserelease = false;
> +    bool is_etc_osrelease   = false;
> +    if (strcmp(file, "/etc/SuSE-release") == 0) {
> +      is_etc_suserelease = true;
> +    }
> +    if (strcmp(file, "/etc/os-release") == 0) {
> +      is_etc_osrelease = true;
> +    }
> +
> +    // get last line of the file or
> +    // other interesting line on SUSE / os-release
> +    while (fgets(buf, sizeof(buf), fp)) {
> +      if (lcnt == 0 && is_etc_suserelease) {
> +        break;
> +      }
> +      if (is_etc_osrelease) {
> +        if (strstr(buf, "PRETTY_NAME=") != NULL) {
> +          break;
> +        }
> +      }
> +      lcnt++;
> +    }
> +
>       // Edit out extra stuff in expected ubuntu format
> -    if (strstr(buf, "DISTRIB_DESCRIPTION=") != NULL) {
> +    if (strstr(buf, "DISTRIB_DESCRIPTION=") != NULL || strstr(buf, "PRETTY_NAME=") != NULL) {
>         char* ptr = strstr(buf, "\"");  // the name is in quotes
>         if (ptr != NULL) {
>           ptr++; // go beyond first quote