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

[Coleen Phillimore]

Hi Matthais, I captured this email in bug and will fix the indentation 
and will send this out as both of us as contributors.

https://bugs.openjdk.java.net/browse/JDK-8154566

So this works properly on SuSE?

Thanks!
Coleen

On 4/19/16 10:46 AM, Baesken, Matthias wrote:
>
> Ø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 
> <http://cr.openjdk.java.net/%7Ecoleenp/suse/webrev>
>
> Ø
>
> ØColeen
>
> Hello Coleen,  thanks a lot for creating the webrev,  I think it looks 
> good (better than original diff)    with the exception of
>
> the indentation at the beginning  of   parse_os_info_helper  (buf 
> declaration) .
>
> 2065 static void parse_os_info_helper(FILE* fp, char* distro, size_t 
> length, bool get_first_line) {
>
> 2066     char buf[256];
>
> 2067   while (fgets(buf, sizeof(buf), fp)) {
>
> Best regards, Matthias
>
> --------------------------------------------------------------
>
> 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
>