* Cleanups to grfio.

- Replaced strncpy with safestrncpy (bugreport:3080).
- Ensured, that all local paths are normalized and work whether or not the data dir ends with '/'.
- Local files are no longer added to the GRF+alias file list (apparently served to cache file size, with no performance gain).
- Buffer for files is no longer allocated with 1024 extra bytes, but only 1 for zero-termination of text files.

git-svn-id: https://svn.code.sf.net/p/rathena/svn/trunk@14647 54d463be-8e91-2dee-dedb-b68131a5f0ec

Changelog-Trunk.txt

@@ -1,6 +1,11 @@
 Date	Added
 
 2011/01/01
+	* Cleanups to grfio. [Ai4rei]
+	- Replaced strncpy with safestrncpy (bugreport:3080).
+	- Ensured, that all local paths are normalized and work whether or not the data dir ends with '/'.
+	- Local files are no longer added to the GRF+alias file list (apparently served to cache file size, with no performance gain).
+	- Buffer for files is no longer allocated with 1024 extra bytes, but only 1 for zero-termination of text files.
 	* Updates to map cache generator tool. [Ai4rei]
 	- Removed unmaintained grfio library copy and made the tool use the one in /common instead (related r12726).
 	- Updated makefile to use compile options/libraries determined by configure (bugreport:1109).

src/common/grfio.c

@@ -12,6 +12,7 @@
 #include "../common/cbasetypes.h"
 #include "../common/showmsg.h"
 #include "../common/malloc.h"
+#include "../common/strlib.h"
 
 
 //----------------------------
@@ -381,6 +382,35 @@ static void filelist_adjust(void)
 	}
 }
 
+
+/// Combines are resource path with the data folder location to
+/// create local resource path.
+static void grfio_localpath_create(char* buffer, size_t size, const char* filename)
+{
+	unsigned int i;
+	size_t len;
+
+	len = strlen(data_dir);
+
+	if( data_dir[0] == 0 || data_dir[len-1] == '/' || data_dir[len-1] == '\\' )
+	{
+		safesnprintf(buffer, size, "%s%s", data_dir, filename);
+	}
+	else
+	{
+		safesnprintf(buffer, size, "%s/%s", data_dir, filename);
+	}
+
+	for( i = 0; buffer[i]; i++ )
+	{// normalize path
+		if( buffer[i] == '\\' )
+		{
+			buffer[i] = '/';
+		}
+	}
+}
+
+
 /***********************************************************
  ***                  Grfio Sobroutines                  ***
  ***********************************************************/
@@ -398,13 +428,10 @@ int grfio_size(char* fname)
 		FILELIST lentry;
 		struct stat st;
 
-		sprintf(lfname, "%s%s", data_dir, fname);
-
-		for (p = &lfname[0]; *p != 0; p++)
-			if (*p=='\\') *p = '/';
+		grfio_localpath_create(lfname, sizeof(lfname), fname);
 
 		if (stat(lfname, &st) == 0) {
-			strncpy(lentry.fn, fname, sizeof(lentry.fn) - 1);
+			safestrncpy(lentry.fn, fname, sizeof(lentry.fn));
 			lentry.fnd = NULL;
 			lentry.declen = st.st_size;
 			lentry.gentry = 0;	// 0:LocalFile
@@ -428,35 +455,28 @@ void* grfio_reads(char* fname, int* size)
 	entry = filelist_find(fname);
 
 	if (entry == NULL || entry->gentry <= 0) {	// LocalFileCheck
-		char lfname[256], *p;
-		FILELIST lentry;
+		char lfname[256];
+		int declen;
 
-		sprintf(lfname, "%s%s", data_dir, fname);
-		
-		for (p = &lfname[0]; *p != 0; p++)
-			if (*p == '\\') *p = '/';
+		grfio_localpath_create(lfname, sizeof(lfname), fname);
 
 		in = fopen(lfname, "rb");
 		if (in != NULL) {
-			if (entry != NULL && entry->gentry == 0) {
-				lentry.declen = entry->declen;
-			} else {
-				fseek(in,0,SEEK_END);
-				lentry.declen = ftell(in);
-			}
+			fseek(in,0,SEEK_END);
+			declen = ftell(in);
 			fseek(in,0,SEEK_SET);
-			buf2 = (unsigned char *)aMallocA(lentry.declen + 1024);
-			fread(buf2, 1, lentry.declen, in);
+			buf2 = (unsigned char *)aMallocA(declen+1);  // +1 for resnametable zero-termination
+			fread(buf2, 1, declen, in);
 			fclose(in);
-			strncpy(lentry.fn, fname, sizeof(lentry.fn) - 1);
-			lentry.fnd = NULL;
-			lentry.gentry = 0;	// 0:LocalFile
-			entry = filelist_modify(&lentry);
+			if( size )
+			{
+				size[0] = declen;
+			}
 		} else {
 			if (entry != NULL && entry->gentry < 0) {
 				entry->gentry = -entry->gentry;	// local file checked
 			} else {
-				ShowError("%s not found (grfio_reads - local file %s)\n", fname, lfname);
+				ShowError("grfio_reads: %s not found (local file: %s)\n", fname, lfname);
 				return NULL;
 			}
 		}
@@ -465,11 +485,11 @@ void* grfio_reads(char* fname, int* size)
 		char* grfname = gentry_table[entry->gentry - 1];
 		in = fopen(grfname, "rb");
 		if(in != NULL) {
-			unsigned char *buf = (unsigned char *)aMallocA(entry->srclen_aligned + 1024);
+			unsigned char *buf = (unsigned char *)aMallocA(entry->srclen_aligned);
 			fseek(in, entry->srcpos, 0);
 			fread(buf, 1, entry->srclen_aligned, in);
 			fclose(in);
-			buf2 = (unsigned char *)aMallocA(entry->declen + 1024);
+			buf2 = (unsigned char *)aMallocA(entry->declen+1);  // +1 for resnametable zero-termination
 			if (entry->type == 1 || entry->type == 3 || entry->type == 5) {
 				uLongf len;
 				if (entry->cycle >= 0)
@@ -485,14 +505,16 @@ void* grfio_reads(char* fname, int* size)
 			} else {
 				memcpy(buf2, buf, entry->declen);
 			}
+			if( size )
+			{
+				size[0] = entry->declen;
+			}
 			aFree(buf);
 		} else {
-			ShowError("%s not found (grfio_reads - GRF file %s)\n", fname, grfname);
+			ShowError("grfio_reads: %s not found (GRF file: %s)\n", fname, grfname);
 			return NULL;
 		}
 	}
-	if (size != NULL && entry != NULL)
-		*size = entry->declen;
 
 	return buf2;
 }
@@ -590,7 +612,7 @@ static int grfio_entryread(char* grfname, int gentry)
 				aentry.srcpos         = getlong(grf_filelist+ofs2+13)+0x2e;
 				aentry.cycle          = srccount;
 				aentry.type           = type;
-				strncpy(aentry.fn, fname,sizeof(aentry.fn)-1);
+				safestrncpy(aentry.fn, fname, sizeof(aentry.fn));
 				aentry.fnd			  = NULL;
 #ifdef	GRFIO_LOCAL
 				aentry.gentry         = -(gentry+1);	// As Flag for making it a negative number carrying out the first time LocalFileCheck
@@ -657,7 +679,7 @@ static int grfio_entryread(char* grfname, int gentry)
 				aentry.srcpos         = getlong(grf_filelist+ofs2+13)+0x2e;
 				aentry.cycle          = srccount;
 				aentry.type           = type;
-				strncpy(aentry.fn,fname,sizeof(aentry.fn)-1);
+				safestrncpy(aentry.fn, fname, sizeof(aentry.fn));
 				aentry.fnd			  = NULL;
 #ifdef	GRFIO_LOCAL
 				aentry.gentry         = -(gentry+1);	// As Flag for making it a negative number carrying out the first time LocalFileCheck
@@ -694,9 +716,7 @@ static void grfio_resourcecheck(void)
 	int i = 0;
 
 	// read resnametable from data directory and return if successful
-	sprintf(restable, "%sdata\\resnametable.txt", data_dir);
-	for (ptr = &restable[0]; *ptr != 0; ptr++)
-		if (*ptr == '\\') *ptr = '/';
+	grfio_localpath_create(restable, sizeof(restable), "data\\resnametable.txt");
 
 	fp = fopen(restable, "rb");
 	if (fp) {
@@ -710,10 +730,11 @@ static void grfio_resourcecheck(void)
 				sprintf(dst, "data\\%s", w2);
 				entry = filelist_find(dst);
 				// create new entries reusing the original's info
-				if (entry != NULL) {
+				if (entry != NULL)
+				{// alias for GRF resource
 					FILELIST fentry;
 					memcpy(&fentry, entry, sizeof(FILELIST));
-					strncpy(fentry.fn, src, sizeof(fentry.fn) - 1);
+					safestrncpy(fentry.fn, src, sizeof(fentry.fn));
 					fentry.fnd = aStrdup(dst);
 					filelist_modify(&fentry);
 					i++;
@@ -738,10 +759,11 @@ static void grfio_resourcecheck(void)
 				sprintf(src, "data\\%s", w1);
 				sprintf(dst, "data\\%s", w2);
 				entry = filelist_find(dst);
-				if (entry != NULL) {
+				if (entry != NULL)
+				{// alias for GRF resource
 					FILELIST fentry;
 					memcpy(&fentry, entry, sizeof(FILELIST));
-					strncpy(fentry.fn, src, sizeof(fentry.fn) - 1);
+					safestrncpy(fentry.fn, src, sizeof(fentry.fn));
 					fentry.fnd = aStrdup(dst);
 					filelist_modify(&fentry);
 					i++;
@@ -824,7 +846,7 @@ void grfio_init(char* fname)
 			if(strcmp(w1, "grf") == 0)	// GRF file
 				grf_num += (grfio_add(w2) == 0);
 			else if(strcmp(w1,"data_dir") == 0) {	// Data directory
-				strcpy(data_dir, w2);
+				safestrncpy(data_dir, w2, sizeof(data_dir));
 			}
 		}
 		fclose(data_conf);