Surface: Load image with factory method instead of constructor.

If loading fails, the factory method returns NULL, while previously the
constructor would create a Surface object with a NULL "raw" field.
However, since most of the methods dereference "raw" without checking,
such a Surface would likely crash the application when used.

src/gmenu2x.cpp

@@ -351,7 +351,7 @@ void GMenu2X::initBG() {
 		bg->box(0,0,resX,resY,0,0,0);
 	} else {
 		// Note: Copy constructor converts to display format.
-		bg = new Surface(Surface(confStr["wallpaper"]));
+		bg = new Surface(Surface::loadImage(confStr["wallpaper"]));
 	}
 
 	drawTopBar(bg);
@@ -360,37 +360,46 @@ void GMenu2X::initBG() {
 	Surface *bgmain = new Surface(bg);
 	sc.add(bgmain,"bgmain");
 
-	Surface sd("imgs/sd.png", confStr["skin"]);
+	Surface *sd = Surface::loadImage("imgs/sd.png", confStr["skin"]);
-	Surface cpu("imgs/cpu.png", confStr["skin"]);
+	if (sd) sd->blit(sc["bgmain"], 3, bottomBarIconY);
-	Surface volume("imgs/volume.png", confStr["skin"]);
 	string df = getDiskFree();
+	sc["bgmain"]->write(font, df, 22, bottomBarTextY, ASFont::HAlignLeft, ASFont::VAlignMiddle);
+	free(sd);
 
-	sd.blit( sc["bgmain"], 3, bottomBarIconY );
+	Surface *volume = Surface::loadImage("imgs/volume.png", confStr["skin"]);
-	sc["bgmain"]->write( font, df, 22, bottomBarTextY, ASFont::HAlignLeft, ASFont::VAlignMiddle );
 	volumeX = 27+font->getTextWidth(df);
-	volume.blit( sc["bgmain"], volumeX, bottomBarIconY );
+	if (volume) volume->blit(sc["bgmain"], volumeX, bottomBarIconY);
 	volumeX += 19;
+	free(volume);
+
+	Surface *cpu = Surface::loadImage("imgs/cpu.png", confStr["skin"]);
 	cpuX = volumeX+font->getTextWidth("100")+5;
-	cpu.blit( sc["bgmain"], cpuX, bottomBarIconY );
+	if (cpu) cpu->blit(sc["bgmain"], cpuX, bottomBarIconY);
 	cpuX += 19;
 	manualX = cpuX+font->getTextWidth("300Mhz")+5;
+	free(cpu);
 
 	int serviceX = resX-38;
 	if (usbnet) {
 		if (web) {
-			Surface webserver("imgs/webserver.png", confStr["skin"]);
+			Surface *webserver = Surface::loadImage(
-			webserver.blit( sc["bgmain"], serviceX, bottomBarIconY );
+				"imgs/webserver.png", confStr["skin"]);
+			if (webserver) webserver->blit(sc["bgmain"], serviceX, bottomBarIconY);
 			serviceX -= 19;
+			free(webserver);
 		}
 		if (samba) {
-			Surface sambaS("imgs/samba.png", confStr["skin"]);
+			Surface *sambaS = Surface::loadImage(
-			sambaS.blit( sc["bgmain"], serviceX, bottomBarIconY );
+				"imgs/samba.png", confStr["skin"]);
+			if (sambaS) sambaS->blit(sc["bgmain"], serviceX, bottomBarIconY);
 			serviceX -= 19;
+			free(sambaS);
 		}
 		if (inet) {
-			Surface inetS("imgs/inet.png", confStr["skin"]);
+			Surface *inetS = Surface::loadImage("imgs/inet.png", confStr["skin"]);
-			inetS.blit( sc["bgmain"], serviceX, bottomBarIconY );
+			if (inetS) inetS->blit(sc["bgmain"], serviceX, bottomBarIconY);
 			serviceX -= 19;
+			free(inetS);
 		}
 	}
 }

src/linkapp.cpp

@@ -300,14 +300,17 @@ void LinkApp::showManual() {
 		//Raise the clock to speed-up the loading of the manual
 		gmenu2x->setClock(336);
 
-		Surface pngman(manual);
+		Surface *pngman = Surface::loadImage(manual);
+		if (!pngman) {
+			return;
+		}
 		// Note: Copy constructor converts to display format.
-		Surface bg(Surface(gmenu2x->confStr["wallpaper"]));
+		Surface bg(Surface::loadImage(gmenu2x->confStr["wallpaper"]));
 		stringstream ss;
 		string pageStatus;
 
 		bool close = false, repaint = true;
-		int page=0, pagecount=pngman.raw->w/320;
+		int page=0, pagecount=pngman->raw->w/320;
 
 		ss << pagecount;
 		string spagecount;
@@ -319,7 +322,7 @@ void LinkApp::showManual() {
 		while (!close) {
 			if (repaint) {
 				bg.blit(gmenu2x->s, 0, 0);
-				pngman.blit(gmenu2x->s, -page*320, 0);
+				pngman->blit(gmenu2x->s, -page*320, 0);
 
 				gmenu2x->drawBottomBar();
 				gmenu2x->drawButton(gmenu2x->s, "x", gmenu2x->tr["Exit"],

src/surface.cpp

@@ -44,10 +44,31 @@ Surface *Surface::openOutputSurface(int width, int height, int bitsperpixel) {
 	return raw ? new Surface(raw, false) : NULL;
 }
 
+Surface *Surface::loadImage(const string &img, const string &skin) {
+	string skinpath;
+	if (!skin.empty() && !img.empty() && img[0]!='/') {
+		skinpath = "skins/"+skin+"/"+img;
+		if (!fileExists(skinpath))
+			skinpath = "skins/Default/"+img;
+	} else {
+		skinpath = img;
+	}
+
+	SDL_Surface *raw = loadPNG(skinpath);
+	if (!raw) {
+		ERROR("Couldn't load surface '%s'\n", img.c_str());
+		return NULL;
+	}
+
+	return new Surface(raw, true);
+}
+
 Surface::Surface(SDL_Surface *raw_, bool freeWhenDone_)
 	: raw(raw_)
 	, freeWhenDone(freeWhenDone_)
 {
+	halfW = raw->w/2;
+	halfH = raw->h/2;
 }
 
 Surface::Surface(Surface *s) {
@@ -57,14 +78,6 @@ Surface::Surface(Surface *s) {
 	halfH = raw->h/2;
 }
 
-Surface::Surface(const string &img, const string &skin) {
-	raw = NULL;
-	load(img, skin);
-	freeWhenDone = (raw != NULL);
-	halfW = raw->w/2;
-	halfH = raw->h/2;
-}
-
 Surface::~Surface() {
 	if (freeWhenDone) {
 		SDL_FreeSurface(raw);
@@ -78,22 +91,6 @@ SDL_PixelFormat *Surface::format() {
 		return raw->format;
 }
 
-void Surface::load(const string &img, const string &skin) {
-	string skinpath;
-	if (!skin.empty() && !img.empty() && img[0]!='/') {
-		skinpath = "skins/"+skin+"/"+img;
-		if (!fileExists(skinpath))
-			skinpath = "skins/Default/"+img;
-	} else {
-		skinpath = img;
-	}
-
-	raw = loadPNG(skinpath);
-	if (!raw) {
-		ERROR("Couldn't load surface '%s'\n", img.c_str());
-	}
-}
-
 void Surface::flip() {
 	SDL_Flip(raw);
 }

src/surface.h

@@ -40,9 +40,9 @@ RGBAColor strtorgba(const string &strColor);
 class Surface {
 public:
 	static Surface *openOutputSurface(int width, int height, int bitsperpixel);
+	static Surface *loadImage(const string &img, const string &skin="");
 
 	Surface(Surface *s);
-	Surface(const string &img, const string &skin="");
 	~Surface();
 
 	SDL_Surface *raw;
@@ -74,7 +74,6 @@ public:
 private:
 	Surface(SDL_Surface *raw, bool freeWhenDone);
 	SDL_PixelFormat *format();
-	void load(const string &img, const string &skin);
 	bool blit(SDL_Surface *destination, int x, int y, int w=0, int h=0, int a=-1);
 	bool blitCenter(SDL_Surface *destination, int x, int y, int w=0, int h=0, int a=-1);
 	bool blitRight(SDL_Surface *destination, int x, int y, int w=0, int h=0, int a=-1);

src/surfacecollection.cpp

@@ -80,8 +80,10 @@ Surface *SurfaceCollection::add(const string &path) {
 			return NULL;
 	} else if (!fileExists(filePath)) return NULL;
 
-	Surface *s = new Surface(filePath);
+	Surface *s = Surface::loadImage(filePath);
-	surfaces[path] = s;
+	if (s != NULL) {
+		surfaces[path] = s;
+	}
 	return s;
 }
 
@@ -94,9 +96,10 @@ Surface *SurfaceCollection::addSkinRes(const string &path) {
 	string skinpath = getSkinFilePath(path);
 	if (skinpath.empty())
 		return NULL;
-	Surface *s = new Surface(skinpath);
+	Surface *s = Surface::loadImage(skinpath);
-	if (s != NULL)
+	if (s != NULL) {
 		surfaces[path] = s;
+	}
 	return s;
 }