#exult@irc.freenode.net logs for 3 Dec 2012 (GMT)

Archive Today Yesterday Tomorrow
Exult homepage


[00:47:42] --- Sevalecan is now known as Sevalefonz
[04:10:15] --> fearyourself has joined #exult
[04:10:23] <fearyourself> good evening
[04:11:26] <fearyourself> Index: tools/splitshp.cc
[04:11:26] <fearyourself> ===================================================================
[04:11:26] <fearyourself> --- tools/splitshp.cc (revision 7169)
[04:11:26] <fearyourself> +++ tools/splitshp.cc (working copy)
[04:11:26] <fearyourself> @@ -20,6 +20,7 @@
[04:11:27] <fearyourself> # include <config.h>
[04:11:29] <fearyourself> #endif
[04:11:31] <fearyourself>
[04:11:33] <fearyourself> +#include <cassert>
[04:11:35] <fearyourself> #include <iostream>
[04:11:39] <fearyourself> #include <cstdlib>
[04:11:41] <fearyourself>
[04:11:43] <fearyourself> @@ -129,6 +130,7 @@
[04:11:45] <fearyourself> uint8 *data;
[04:11:47] <fearyourself> char *framename;
[04:11:49] <fearyourself> int i;
[04:11:51] <fearyourself> + int err;
[04:11:53] <fearyourself>
[04:11:55] <fearyourself> shpfile = fopen (filename, "rb");
[04:11:57] <fearyourself> if (!shpfile) {
[04:11:59] <fearyourself> @@ -150,7 +152,8 @@
[04:12:01] <fearyourself> framename = framefilename(filename, i);
[04:12:03] <fearyourself> cout << "writing " << framename << "..." << endl;
[04:12:05] <fearyourself> framefile = fopen(framename, "wb");
[04:12:09] <fearyourself> - fread(data, 1, 64, shpfile);
[04:12:11] <fearyourself> + err = fread(data, 1, 64, shpfile);
[04:12:13] <fearyourself> + assert (err == 64);
[04:12:15] <fearyourself> fwrite(data, 1, 64, framefile);
[04:12:17] <fearyourself> fclose(framefile);
[04:12:19] <fearyourself> delete[] framename;
[04:12:21] <fearyourself> @@ -183,7 +186,8 @@
[04:12:23] <fearyourself> write4(framefile, 8);
[04:12:25] <fearyourself>
[04:12:27] <fearyourself> data = new uint8[datalen];
[04:12:29] <fearyourself> - fread(data, 1, datalen, shpfile);
[04:12:31] <fearyourself> + err = fread(data, 1, datalen, shpfile);
[04:12:31] <sh4rm4> ever heard of pastebin ?
[04:12:33] <fearyourself> + assert (err == datalen);
[04:12:35] <fearyourself> fwrite(data, 1, datalen, framefile);
[04:12:39] <fearyourself> fclose(framefile);
[04:12:41] <fearyourself> delete[] framename;
[04:12:43] <fearyourself> @@ -203,6 +207,7 @@
[04:12:45] <fearyourself> int file_size, shape_size, hdr_size, frame_size;
[04:12:47] <fearyourself> int total_size;
[04:12:49] <fearyourself> uint8 *data;
[04:12:51] <fearyourself> + int err;
[04:12:53] <fearyourself>
[04:12:55] <fearyourself> shpfile = fopen(shapefile, "wb");
[04:12:57] <fearyourself>
[04:12:57] <sh4rm4> stop it
[04:12:59] <fearyourself> @@ -231,7 +236,8 @@
[04:13:01] <fearyourself>
[04:13:03] <fearyourself> data = new uint8[64];
[04:13:05] <fearyourself>
[04:13:09] <fearyourself> - fread(data, 1, 64, framefile);
[04:13:11] <fearyourself> + err = fread(data, 1, 64, framefile);
[04:13:13] <fearyourself> + assert (err == 64);
[04:13:15] <fearyourself> fwrite(data, 1, 64, shpfile);
[04:13:17] <fearyourself> fclose(framefile);
[04:13:18] <sh4rm4> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
[04:13:19] <fearyourself>
[04:13:21] <fearyourself> @@ -253,7 +259,8 @@
[04:13:23] <fearyourself>
[04:13:25] <fearyourself> data = new uint8[frame_size];
[04:13:27] <fearyourself> fseek(framefile, hdr_size, SEEK_SET);
[04:13:29] <fearyourself> - fread(data, 1, frame_size, framefile);
[04:13:31] <fearyourself> + err = fread(data, 1, frame_size, framefile);
[04:13:33] <fearyourself> + assert (err == frame_size);
[04:13:35] <fearyourself> fclose(framefile);
[04:13:39] <fearyourself>
[04:13:41] <fearyourself> fseek(shpfile, 4 + (i*4), SEEK_SET);
[04:13:43] <fearyourself> Index: tools/expack.cc
[04:13:45] <fearyourself> ===================================================================
[04:13:47] <fearyourself> --- tools/expack.cc (revision 7169)
[04:13:49] <fearyourself> +++ tools/expack.cc (working copy)
[04:13:51] <fearyourself> @@ -297,14 +297,14 @@
[04:13:53] <fearyourself> // Read the output file name
[04:13:55] <fearyourself> getline(respfile, temp, 1024);
[04:13:57] <fearyourself> strncpy(fname, path_prefix, 1024);
[04:13:59] <fearyourself> - strncat(fname, temp, 10
[04:14:01] <fearyourself> ahh not enough space
[04:14:03] <fearyourself> I removed all the warnings from the current trunk of exult
[04:14:05] <fearyourself> I'll find another way of sending it to you guys :-)
[04:14:09] <fearyourself> ;-)
[04:14:11] <fearyourself> http://pastebin.com/fxg2iPQ4
[04:14:13] <fearyourself> there we go
[04:14:15] <fearyourself> and no I hadn't
[04:14:17] <fearyourself> :-)
[04:14:51] <sh4rm4> noone's going to rip a patch out of a chatlog ever
[04:15:01] <fearyourself> well there ya go, I fixed all the warnings the compilation process gives me
[04:15:36] <sh4rm4> it is wrong
[04:15:44] <sh4rm4> int is not the correct type
[04:16:07] <sh4rm4> man fread
[04:16:13] <sh4rm4> size_t fread(void *ptr, size_t size, size_t nmemb, FILE *stream);
[04:16:50] <sh4rm4> also i doubt that your strncat change doesnt break stuff
[04:17:07] <fearyourself> fair enough for the size_t, I generally do int, but I agree
[04:17:12] <fearyourself> why for the strncat ?
[04:17:15] <sh4rm4> and wtf is cassert ?
[04:17:21] <sh4rm4> is that some windows crap ?
[04:17:25] <fearyourself> at best, it is correct, at worse it was overflowing
[04:17:32] <fearyourself> cassert is C++'s version of assert
[04:17:52] <sh4rm4> only in windows land ?
[04:18:02] <sh4rm4> do you know what strncpy does ?
[04:18:27] <fearyourself> no in C++'s norm
[04:18:32] <fearyourself> I don't work under windows
[04:18:38] <fearyourself> and yes I do
[04:18:50] <sh4rm4> then please explain
[04:19:29] <fearyourself> you'll notice I don't change strncpy but only strncat calls
[04:19:53] <fearyourself> but to answer your question, strncpy copies from src to dest only the n first characters or when
[04:19:55] <fearyourself> \0 is found
[04:20:02] <sh4rm4> no
[04:20:26] <sh4rm4> it copies the str until \0 is found, and fills the remaining space with 0's
[04:20:43] <sh4rm4> so it always writes exactly n bytes
[04:21:01] <sh4rm4> anyway
[04:21:13] <sh4rm4> your patch uses int where it should use size_t
[04:21:19] <sh4rm4> it breaks indentation
[04:22:03] <fearyourself> fair enough
[04:22:06] <fearyourself> The strncpy() function is similar, except that at most n bytes of src are copied. Warning: If there is no null byte
[04:22:07] <fearyourself> among the first n bytes of src, the string placed in dest will not be null terminated.
[04:22:07] <fearyourself> If the length of src is less than n, strncpy() pads the remainder of dest with null bytes.
[04:22:24] <fearyourself> never paid attention to the padding
[04:22:37] <fearyourself> what part breaks it?
[04:22:54] <sh4rm4> that makes strncpy more or less useless, unless you use it to fill block-based record data
[04:23:20] <sh4rm4> what you usually want when you use strncpy is snprintf(dest, n, "%s", yourstring)
[04:24:14] <fearyourself> I haven't added any strncpy to the code, I only fixed the N part for strcats...
[04:24:24] <sh4rm4> yes
[04:24:27] <fearyourself> which provokes an overflow risk
[04:24:39] <sh4rm4> your patch uses different indentation than the surrending code
[04:25:13] <sh4rm4> the exult indentation is as bad as it can get, but the code should still stay consistent
[04:26:01] <sh4rm4> imo it should be reformatted once with a proper tool in a single commit, that uses tabs instead of spaces and removes the extremely verbose bracing style
[04:27:05] <fearyourself> well from what I see it has a mix of tabs and spaces thus why my patch shows issues, I'm correcting that
[04:28:05] <sh4rm4> strncpy (hprefix, fname, 1024);
[04:28:05] <sh4rm4> make_header_name(hprefix);
[04:28:05] <sh4rm4> - strncat (hname, hprefix, 1024);
[04:28:05] <sh4rm4> - strncat (hname, ".h", 1024);
[04:28:05] <sh4rm4> + strncat (hname, hprefix, sizeof (hname) - strlen (hname));
[04:28:06] <sh4rm4> + strncat (hname, ".h", sizeof (hname) - strlen (hname));
[04:28:44] <sh4rm4> what this code should do is get the length of fname, store it into a variable
[04:28:49] <sh4rm4> then use memcpy
[04:29:11] <sh4rm4> and then another memcpy to hname + len
[04:29:21] <sh4rm4> and then one to hname + len + 2
[04:29:53] <sh4rm4> this way of calling strlen all over is wasteful
[04:31:07] <fearyourself> I really fixed the overflows, better a correct code than a wrong code... then you are right, we could do it better
[04:33:00] <sh4rm4> note that strcat has to do strlen as well in order to work
[04:33:04] <sh4rm4> and strcpy
[04:33:27] <sh4rm4> so you have 4 lines that call strlen 6 times altogether
[04:33:35] <sh4rm4> where a single call would suffice
[04:34:11] <fearyourself> I'm not sure strcpy does a strlen but strcat would
[04:34:21] <sh4rm4> it doesnt directly
[04:34:31] <sh4rm4> but it has to check for zero byte
[04:34:41] <sh4rm4> so it cant work as efficient as memcpy
[04:34:57] <sh4rm4> which could for example copy 8-16 bytes at once
[04:37:58] <fearyourself> agreed
[04:39:48] <sh4rm4> btw
[04:40:33] <sh4rm4> i guess snprintf(hname, 1024, "%s%s.h", fname, hprefix) would work as well
[04:40:53] <sh4rm4> in one line, and relatively effective
[04:42:23] <sh4rm4> it would be a bit slower as the strlen + memcpy variant
[04:42:42] <sh4rm4> but much less verbose, and you dont have to check the remaining space at all
[04:45:42] <sh4rm4> oh wait, fname is copied into hprefix
[04:49:06] <fearyourself> yeah, I did: http://pastebin.com/3kgbBPTz
[04:49:40] <fearyourself> - strncpy (hprefix, fname, 1024);
[04:49:41] <fearyourself> + strncpy (hprefix, fname, sizeof (hprefix));
[04:49:41] <fearyourself> make_header_name(hprefix);
[04:49:41] <fearyourself> - strncat (hname, hprefix, 1024);
[04:49:41] <fearyourself> - strncat (hname, ".h", 1024);
[04:49:41] <fearyourself> + snprintf (hname, sizeof (hname), "%s.h", hprefix);
[04:50:52] <sh4rm4> yeah that looks much better now
[04:51:08] <sh4rm4> except:
[04:51:09] <sh4rm4> unsigned short* pextern;
[04:51:09] <sh4rm4> + size_t err;
[04:51:20] <fearyourself> ah one might have passed through
[04:52:21] <fearyourself> I see another one
[04:52:51] <sh4rm4> why did you remove free_linkedlist and write_datablock ?
[04:54:33] <fearyourself> they were unused, it was a warning
[04:54:43] <fearyourself> like I said, I removed any warning the compilation framework did
[04:54:47] <fearyourself> s/did/said
[04:55:31] <sh4rm4> hmm seems like a bug when free_linkedlist is unused, but other linked_list code is still active
[04:55:42] <sh4rm4> i.e. resources are not freed
[04:55:49] <fearyourself> http://pastebin.com/f5YGjWF4
[04:55:53] <fearyourself> is the latest
[04:56:05] <sh4rm4> anyway: i'd rather #if 0 the code instead of deleting it
[04:56:16] <sh4rm4> and add a comment it is unused
[04:57:49] <sh4rm4> (most ppl dont like when their code gets deleted)
[04:59:27] <fearyourself> ok, my theory is that it is in the svn history with a good SVN message, it is better
[04:59:41] <fearyourself> if 0 tend to bloat code and make code reviews and reading more difficult
[05:00:36] <fearyourself> http://pastebin.com/BqF9p7nW
[05:00:38] <fearyourself> latest version
[05:00:40] <fearyourself> ;-)
[05:01:10] <sh4rm4> good, now send it to the mailing list
[05:01:24] <sh4rm4> or wait until wjp shows up
[05:01:29] <sh4rm4> and ask him to merge
[05:02:57] <fearyourself> I don't even know where the mailing list is, I just put it on ultimacodex where I generally hang out in a thread about exult
[05:03:02] <fearyourself> and exult getting help
[05:03:06] <fearyourself> it'll get found
[05:03:09] <fearyourself> thanks for your help
[05:03:43] <sh4rm4> i'll ask wjp if he can review it
[05:03:52] <sh4rm4> drop your contact address somewhere tho
[05:04:45] <fearyourself> Dominus knows how to contact me, or http://ultimacodex.com/2012/11/exult-help-wanted-and-news/
[05:04:53] <fearyourself> has my info under jc/fearyourself
[05:04:57] <sh4rm4> alright
[05:08:01] <fearyourself> do you work on the project as well and if so, what do you do ?
[05:11:33] <sh4rm4> my contribution is to stir up the guys here to keep working on the project
[05:11:44] <sh4rm4> additionally i host a svn mirror on github
[05:12:02] <fearyourself> gotcha :-)
[06:05:41] <fearyourself> I was even smart about it: https://sourceforge.net/tracker/?func=detail&aid=3591998&group_id=2335&atid=302335
[06:06:50] <fearyourself> ok, I have to go, talk to you later
[06:06:58] <-- fearyourself has left IRC (Quit: Leaving)
[09:45:52] --> Rottingbeef has joined #exult
[13:11:19] <-- Kirben has left IRC ()
[13:26:13] --> TheCycoONE has joined #exult
[13:26:15] --> Marzo has joined #exult
[13:31:24] --> Marzo1 has joined #exult
[13:31:24] <-- Marzo has left IRC (Disconnected by services)
[13:36:41] <-- Marzo1 has left IRC (Ping timeout: 256 seconds)
[13:51:19] --> Marzo has joined #exult
[14:25:34] <-- Marzo has left IRC (Ping timeout: 252 seconds)
[14:26:31] --> Marzo has joined #exult
[15:22:23] <-- Marzo has left IRC (Ping timeout: 252 seconds)
[15:23:32] --> Marzo has joined #exult
[15:43:39] <-- Marzo has left IRC (Ping timeout: 252 seconds)
[15:47:28] --> Marzo has joined #exult
[15:54:39] <-- Marzo has left IRC (Ping timeout: 252 seconds)
[15:55:40] --> Marzo has joined #exult
[16:41:13] <-- Marzo has left IRC (Ping timeout: 252 seconds)
[16:42:05] --> Marzo has joined #exult
[16:45:50] --> Colourle1s has joined #exult
[16:45:50] <-- Colourle1s has left IRC (Changing host)
[16:45:50] --> Colourle1s has joined #exult
[16:45:50] --- ChanServ gives channel operator status to Colourle1s
[16:50:56] <-- Colourless has left IRC (*.net *.split)
[16:51:29] <-- Marzo has left IRC (Ping timeout: 253 seconds)
[16:52:48] --> Marzo has joined #exult
[17:12:01] <-- Marzo has left IRC (Ping timeout: 252 seconds)
[17:13:00] --> Marzo has joined #exult
[17:34:00] <-- Marzo has left IRC (Ping timeout: 252 seconds)
[17:46:43] --> Marzo has joined #exult
[19:03:50] <-- Marzo has left IRC (Ping timeout: 252 seconds)
[20:27:54] --- Sevalefonz is now known as Sevalecan
[21:43:21] --> Marzo has joined #exult
[22:05:30] <-- Rottingbeef has left IRC ()
[22:20:06] <-- TheCycoONE has left IRC (Quit: And then there were n-1)
[23:43:43] <-- SHODAN has left IRC (Read error: Operation timed out)
[23:44:08] --> SHODAN has joined #exult