<URL: http://bugs.freeciv.org/Ticket/Display.html?id=39512 >
- When checking if file to load is bz2 file or not, and it was not,
BZ_FILE was not closed, leading to memory leaks.
- Error checking after every bz2 function call.
- fz_strerror() returns errors as sensible text instead of just
"error number x"
- There was bug in fz_fgets() making it sometimes to return one
garbage byte after file end
- Files of size 0 or 1 bytes (size as in not bzipped files) were not
handled correctly
TODO (but not in this ticket): Reading of bz2 files should have
better implementation. Now we get one byte at the time from
BZ2_bzRead().
- ML
diff -Nurd -X.diff_ignore freeciv/utility/ioz.c freeciv/utility/ioz.c
--- freeciv/utility/ioz.c 2007-08-04 18:36:14.000000000 +0300
+++ freeciv/utility/ioz.c 2007-08-08 21:54:14.000000000 +0300
@@ -47,6 +47,7 @@
#include <bzlib.h>
#endif
+#include "fcintl.h"
#include "log.h"
#include "mem.h"
#include "shared.h"
@@ -60,6 +61,7 @@
FILE *plain;
int error;
int firstbyte;
+ bool eof;
};
#endif
@@ -138,17 +140,43 @@
/* Try to read first byte out of stream so we can figure out if this
really is bzip2 file or not. Store byte for later use */
char tmp;
- BZ2_bzRead(&fp->u.bz2.error, fp->u.bz2.file, &tmp, 1);
- if (fp->u.bz2.error != BZ_DATA_ERROR_MAGIC) { /* bzip2 file */
- if (fp->u.bz2.error != BZ_OK) {
+ int read_len;
+
+ /* We put error to tmp variable when we don't want to overwrite
+ * error already in fp->u.bz2.error. So calls to fz_ferror() or
+ * fz_strerror() will later return what originally went wrong,
+ * and not what happened in error recovery. */
+ int tmp_err;
+
+ read_len = BZ2_bzRead(&fp->u.bz2.error, fp->u.bz2.file, &tmp, 1);
+ if (fp->u.bz2.error != BZ_DATA_ERROR_MAGIC) {
+ /* bzip2 file */
+ if (fp->u.bz2.error == BZ_STREAM_END) {
+ /* We already reached end of file with our read of one byte */
+ if (read_len == 0) {
+ /* 0 byte file */
+ fp->u.bz2.firstbyte = -1;
+ } else {
+ fp->u.bz2.firstbyte = tmp;
+ }
+ fp->u.bz2.eof = TRUE;
+ } else if (fp->u.bz2.error != BZ_OK) {
+ /* Read failed */
+ BZ2_bzReadClose(&tmp_err, fp->u.bz2.file);
fclose(fp->u.bz2.plain);
free(fp);
return NULL;
+ } else {
+ /* Read success and we can continue reading */
+ fp->u.bz2.firstbyte = tmp;
+ fp->u.bz2.eof = FALSE;
}
fp->method = FZ_BZIP2;
- fp->u.bz2.firstbyte = tmp;
return fp;
}
+
+ /* Not bzip2 file */
+ BZ2_bzReadClose(&tmp_err, fp->u.bz2.file);
fclose(fp->u.bz2.plain);
}
#endif
@@ -173,6 +201,12 @@
assert(mode[0] == 'w');
fp->u.bz2.file = BZ2_bzWriteOpen(&fp->u.bz2.error, fp->u.bz2.plain,
compress_level, 1, 15);
+ if (fp->u.bz2.error != BZ_OK) {
+ int tmp_err; /* See comments for similar variable
+ * near BZ2_bzReadOpen() */
+ BZ2_bzWriteClose(&tmp_err, fp->u.bz2.file, 0, NULL, NULL);
+ fp->u.bz2.file = NULL;
+ }
}
if (!fp->u.bz2.file) {
if (fp->u.bz2.plain) {
@@ -289,26 +323,38 @@
case FZ_BZIP2:
{
int i = 0;
+ int last_read;
+
/* See if first byte is already read and stored */
if (fp->u.bz2.firstbyte >= 0) {
buffer[0] = fp->u.bz2.firstbyte;
fp->u.bz2.firstbyte = -1;
i++;
} else {
- BZ2_bzRead(&fp->u.bz2.error, fp->u.bz2.file, buffer + i, 1);
- i++;
- }
- /* Leave space for trailing zero */
- for (; i < size - 1 && fp->u.bz2.error == BZ_OK && buffer[i - 1] != '\n' ;
- i++) {
- BZ2_bzRead(&fp->u.bz2.error, fp->u.bz2.file, buffer + i, 1);
+ if (!fp->u.bz2.eof) {
+ last_read = BZ2_bzRead(&fp->u.bz2.error, fp->u.bz2.file, buffer + i, 1);
+ i += last_read; /* 0 or 1 */
+ }
}
- if (fp->u.bz2.error != BZ_OK &&
- (fp->u.bz2.error != BZ_STREAM_END ||
- i == 0)) {
- retval = NULL;
- } else {
- retval = buffer;
+ if (!fp->u.bz2.eof) {
+ /* Leave space for trailing zero */
+ for (; i < size - 1
+ && fp->u.bz2.error == BZ_OK && buffer[i - 1] != '\n' ;
+ i += last_read) {
+ last_read = BZ2_bzRead(&fp->u.bz2.error, fp->u.bz2.file,
+ buffer + i, 1);
+ }
+ if (fp->u.bz2.error != BZ_OK &&
+ (fp->u.bz2.error != BZ_STREAM_END ||
+ i == 0)) {
+ retval = NULL;
+ } else {
+ retval = buffer;
+ }
+ if (fp->u.bz2.error == BZ_STREAM_END) {
+ /* EOF reached. Do not BZ2_bzRead() any more. */
+ fp->u.bz2.eof = TRUE;
+ }
}
buffer[i] = '\0';
break;
@@ -446,8 +492,66 @@
case FZ_BZIP2:
{
static char bzip2error[50];
- my_snprintf(bzip2error, sizeof(bzip2error), "Bzip2 error %d",
- fp->u.bz2.error);
+ char *cleartext = NULL;
+
+ /* Rationale for translating these:
+ * - Some of them provide usable information to user
+ * - Messages still contain numerical error code for developers
+ */
+ switch(fp->u.bz2.error) {
+ case BZ_OK:
+ cleartext = _("OK");
+ break;
+ case BZ_RUN_OK:
+ cleartext = _("Run ok");
+ break;
+ case BZ_FLUSH_OK:
+ cleartext = _("Flush ok");
+ break;
+ case BZ_FINISH_OK:
+ cleartext = _("Finish ok");
+ break;
+ case BZ_STREAM_END:
+ cleartext = _("Stream end");
+ break;
+ case BZ_CONFIG_ERROR:
+ cleartext = _("Config error");
+ break;
+ case BZ_SEQUENCE_ERROR:
+ cleartext = _("Sequence error");
+ break;
+ case BZ_PARAM_ERROR:
+ cleartext = _("Parameter error");
+ break;
+ case BZ_MEM_ERROR:
+ cleartext = _("Mem error");
+ break;
+ case BZ_DATA_ERROR:
+ cleartext = _("Data error");
+ break;
+ case BZ_DATA_ERROR_MAGIC:
+ cleartext = _("Not bzip2 file");
+ break;
+ case BZ_IO_ERROR:
+ cleartext = _("IO error");
+ break;
+ case BZ_UNEXPECTED_EOF:
+ cleartext = _("Unexpected EOF");
+ break;
+ case BZ_OUTBUFF_FULL:
+ cleartext = _("Output buffer full");
+ break;
+ default:
+ break;
+ }
+
+ if (cleartext != NULL) {
+ my_snprintf(bzip2error, sizeof(bzip2error), _("Bz2: \"%s\" (%d)"),
+ cleartext, fp->u.bz2.error);
+ } else {
+ my_snprintf(bzip2error, sizeof(bzip2error), _("Bz2 error %d"),
+ fp->u.bz2.error);
+ }
retval = bzip2error;
break;
}
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
