Bug 276559 - build failure - gccism
Summary: build failure - gccism
Status: RESOLVED FIXED
Alias: None
Product: ksplash
Classification: Plasma
Component: general (show other bugs)
Version: unspecified
Platform: Compiled Sources Solaris
: NOR normal
Target Milestone: ---
Assignee: Alberto Mattea
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-26 17:55 UTC by tropikhajma
Modified: 2011-08-05 09:49 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.7.0


Attachments
Proposed patch without alloca(3) (1.71 KB, patch)
2011-07-04 04:46 UTC, Raphael Kubo da Costa
Details

Note You need to log in before you can comment on or make changes to this bug.
Description tropikhajma 2011-06-26 17:55:52 UTC
Version:           unspecified (using Devel) 
OS:                Solaris

building kde-workspace 4.6.90 on Solaris 11e with Sun Studio 12u1 fails with:
...
"/export/home/kdebuild/packages/BUILD/kde-workspace-4.6.90/ksplash/ksplashx/main.cpp", line 104: Error: Overloading ambiguity between "std::log10(double)" and "std::log10(float)".
"/export/home/kdebuild/packages/BUILD/kde-workspace-4.6.90/ksplash/ksplashx/main.cpp", line 104: Error: An integer constant expression is required within the array subscript operator.
"/export/home/kdebuild/packages/BUILD/kde-workspace-4.6.90/ksplash/ksplashx/main.cpp", line 105: Error: An integer constant expression is required within the array subscript operator.

This is because variable length arrays are not part of the C++ standard and Studio does not support this gcc-specific extension.

(the first error is trivial, can be fixed with a simple cast)

Reproducible: Always

Steps to Reproduce:
try to build it


Expected Results:  
build fine
Comment 1 tropikhajma 2011-06-26 18:11:06 UTC
this fixed it for me:
--- /export/home/kdebuild/packages/BUILD/kde-workspace-4.6.90/ksplash/ksplashx/main.cpp.orig    2011-06-26 17:43:25.583693929 +0000
+++ /export/home/kdebuild/packages/BUILD/kde-workspace-4.6.90/ksplash/ksplashx/main.cpp 2011-06-26 18:10:30.184073432 +0000
@@ -25,6 +25,7 @@
 #include <errno.h>
 #include <signal.h>
 #include <math.h>
+#include <alloca.h>
 
 int screen_number = 0;
 int number_of_screens = 1;
@@ -101,8 +102,8 @@
                     }
                 // Calculate the array size: part before the dot + length of the screen
                 // string (which is log10 + 1) + 1 for the dot itself + 8 for "DISPLAY=" + \0
-                char envir[breakpos + (int)log10(number_of_screens) + 11];
-                char server_name[breakpos + 1];
+                char *envir; envir = (char*) alloca((breakpos + (int)log10((double)number_of_screens) + 11) * sizeof(*envir));
+                char *server_name; server_name = (char*) alloca((breakpos + 1)*sizeof(char*));
                 strncpy(server_name, display_name, breakpos);
                 server_name[breakpos] = '\0';
Comment 2 Alberto Mattea 2011-06-27 10:59:51 UTC
Thanks for the patch, I'm fine with it. However, although I did the commit that caused this problem, I'm not the maintainer of ksplashx, so I don't know if I can commit it without approvation. Maybe it is better if you create a review request on git.reviewboard.kde.org so that someone who knows this code better than me can review and approve it.
Comment 3 Alberto Mattea 2011-06-27 11:03:23 UTC
On a second note, I've seen Lubos Lunak has assigned it to me, so I'll take it as a permission to commit it myself :-).
Comment 4 Alberto Mattea 2011-06-27 14:31:27 UTC
Git commit 51ff00ae5a0019ecaa5928a05c5e2e427c24d229 by Alberto Mattea.
Committed on 27/06/2011 at 16:28.
Pushed by mattea into branch 'master'.

Fix non-gcc build
Patch by tropikhajma
CCBUG: 276559

M  +4    -2    ksplash/ksplashx/main.cpp     

http://commits.kde.org/kde-workspace/51ff00ae5a0019ecaa5928a05c5e2e427c24d229
Comment 5 Pino Toscano 2011-06-27 14:35:39 UTC
alloca() (and <alloca.h>) is nowhere standard, so please just use standard malloc() + free().
Comment 6 Raphael Kubo da Costa 2011-07-02 21:01:10 UTC
Reopening the report; the use of alloca.h breaks compilation on the BSDs (FreeBSD, OpenBSD, NetBSD and DragonFly BSD), which do not have alloca.h and define alloca(3) in stdlib.h.

Wouldn't it be possible to follow Pino's advice and use malloc()+free()?
Comment 7 Alberto Mattea 2011-07-03 20:13:17 UTC
(In reply to comment #6)
> Reopening the report; the use of alloca.h breaks compilation on the BSDs
> (FreeBSD, OpenBSD, NetBSD and DragonFly BSD), which do not have alloca.h and
> define alloca(3) in stdlib.h.
> 
> Wouldn't it be possible to follow Pino's advice and use malloc()+free()?

I'm fine with malloc, but unfortunately I will not be able to access a machine with git and the other tools until september. So probably it's better if someone else commits the fix.

Thanks
Comment 8 Raphael Kubo da Costa 2011-07-04 04:46:08 UTC
Created attachment 61593 [details]
Proposed patch without alloca(3)

Hajma, can you check if this patch compiles fine for you on Sun Studio?
Comment 9 tropikhajma 2011-07-06 11:25:36 UTC
(In reply to comment #8)
> Created an attachment (id=61593) [details]
> Proposed patch without alloca(3)
> 
> Hajma, can you check if this patch compiles fine for you on Sun Studio?

compiles just fine, thanks!
Comment 10 Raphael Kubo da Costa 2011-07-06 13:10:07 UTC
Git commit 536918d3addabd5ad3613ca8bfc4c0dd224e25b1 by Raphael Kubo da Costa.
Committed on 06/07/2011 at 15:10.
Pushed by rkcosta into branch 'master'.

Fix the build on systems without alloca.h.

As mentioned in the bug report, alloca(3) is not standardized, and some
systems (such as BSDs) do not even have alloca.h. This commit just
replaces its use with new+free.

Verified to build on Sun Studio as well, so the bug remains fixed.

CCBUG: 276559

M  +6    -5    ksplash/ksplashx/main.cpp     

http://commits.kde.org/kde-workspace/536918d3addabd5ad3613ca8bfc4c0dd224e25b1
Comment 11 Christoph Feck 2011-08-04 23:05:05 UTC
Is there anything else that needs to be done for this bug? It is still marked open.
Comment 12 Raphael Kubo da Costa 2011-08-05 09:49:22 UTC
Weird, I thought I had closed it. Doing so now.