I am just returning after a couple years of inactivity and see that development has not been stagnant.
Bravo!
I just did a quick compile of the latest release, however, and found some new warnings (mostly harmless) on the following version of gcc on OS X
Code:
Configured with: --prefix=/Library/Developer/CommandLineTools/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 5.0 (clang-500.2.79) (based on LLVM 3.3svn)
Target: x86_64-apple-darwin13.0.0
Thread model: posix
The fixes are optional, but I personally hate superfluous warnings, as they often draw attention away from ACTUAL errors.
Anyway, here they are. For those who are happy enough to trust me and are not interested in the details, I've attached a patch file to this post.
In fight.c, I'm not sure why dam appears as a dead command after the replace_string() call. I suspect someone
was trying to do something clever, and then forgot to finish. Nonetheless, it should be removed in the in term.
Code:
fight.c
-------
else if (dam <= 23) msgnum = 7;
else msgnum = 8;
/* damage message to onlookers */
buf = replace_string(dam_weapons[msgnum].to_room,
- attack_hit_text[w_type].singular, attack_hit_text[w_type].plural), dam;
+ attack_hit_text[w_type].singular, attack_hit_text[w_type].plural);
act(buf, FALSE, ch, NULL, victim, TO_NOTVICT);
The next issue is an actual error, though it likely would never have caused problems:
The variable 'llen' will always fail to be negative here because it is of type size_t, which is unsigned.
In the rare case where snprintf failed and tried to return a negative value, it would be typecasted into a positive
number when it is assigned to llen, and the error would not be caught.
The following simple modification performs the check correctly.
Code:
dg_olc.c
--------
int format_script(struct descriptor_data *d)
{
char nsc[MAX_CMD_LENGTH], *t, line[READ_SIZE];
char *sc;
- size_t len = 0, nlen = 0, llen = 0;
+ size_t len = 0, nlen = 0, llen = 0, int ret;
int indent = 0, indent_next = FALSE, found_case = FALSE, i, line_num = 0;
if (!d->str || !*d->str)
return FALSE;
sc = strdup(*d->str); /* we work on a copy, because of strtok() */
t = strtok(sc, "\n\r");
*nsc = '\0';
while (t) {
line_num++;
skip_spaces(&t);
if (!strn_cmp(t, "if ", 3) ||
!strn_cmp(t, "switch ", 7)) {
indent_next = TRUE;
} else if (!strn_cmp(t, "while ", 6)) {
found_case = TRUE; /* so you can 'break' a loop without complains */
indent_next = TRUE;
} else if (!strn_cmp(t, "end", 3) ||
!strn_cmp(t, "done", 4)) {
if (!indent) {
write_to_output(d, "Unmatched 'end' or 'done' (line %d)!\r\n", line_num);
free(sc);
return FALSE;
}
indent--;
indent_next = FALSE;
} else if (!strn_cmp(t, "else", 4)) {
if (!indent) {
write_to_output(d, "Unmatched 'else' (line %d)!\r\n", line_num);
free(sc);
return FALSE;
}
indent--;
indent_next = TRUE;
} else if (!strn_cmp(t, "case", 4) ||
!strn_cmp(t, "default", 7)) {
if (!indent) {
write_to_output(d, "Case/default outside switch (line %d)!\r\n", line_num);
free(sc);
return FALSE;
}
if (!found_case) /* so we don't indent multiple case statements without a break */
indent_next = TRUE;
found_case = TRUE;
} else if (!strn_cmp(t, "break", 5)) {
if (!found_case || !indent ) {
write_to_output(d, "Break not in case (line %d)!\r\n", line_num);
free(sc);
return FALSE;
}
found_case = FALSE;
indent--;
}
*line = '\0';
for (nlen = 0, i = 0;i<indent;i++) {
strncat(line, " ", sizeof(line)-1);
nlen += 2;
}
- llen = snprintf(line + nlen, sizeof(line) - nlen, "%s\r\n", t);
+ ret = snprintf(line + nlen, sizeof(line) - nlen, "%s\r\n", t);
+ llen = (size_t)ret;
- if (llen < 0 || llen + nlen + len > d->max_str - 1 ) {
+ if (ret < 0 || llen + nlen + len > d->max_str - 1 ) {
write_to_output(d, "String too long, formatting aborted\r\n");
free(sc);
return FALSE;
}
len = len + nlen + llen;
strcat(nsc, line); /* strcat OK, size checked above */
In objsave.c, it is possible for rentcode to be checked in the switch() case without being initialized.
The mudlog() line I added below can and should be changed to whatever suits the conventions that have been decided upon.
Here is the fix.
Code:
objsave.c
---------
int i, num_of_days, orig_rent_code, num_objs=0;
unsigned long cost;
struct obj_data *cont_row[MAX_BAG_ROWS];
- int rentcode,timed,netcost,gold,account,nitems;
+ int rentcode = RENT_UNDEF, timed, netcost, gold, account, nitems;
obj_save_data *loaded, *current;
if (!get_filename(filename, sizeof(filename), CRASH_FILE, GET_NAME(ch)))
and then lower down
Code:
return 1;
}
if (get_line(fl, line))
- sscanf(line,"%d %d %d %d %d %d",&rentcode, &timed,
- &netcost,&gold,&account,&nitems);
+ mudlog(NRM, MAX(LVL_IMMORT, GET_INVIS_LEV(ch)), TRUE, "Failed to read player's rent code: %s.", GET_NAME(ch));
+ else
+ sscanf(line,"%d %d %d %d %d %d",&rentcode, &timed, &netcost,&gold,&account,&nitems);
if (rentcode == RENT_RENTED || rentcode == RENT_TIMEDOUT) {
sprintf(str, "%d", SECS_PER_REAL_DAY);
The next little change is not an error, but it does produce a warning.
The compiler is complaining that the loop is doing nothing, which is false, because all the work is done in the iterative step, a cute little C technique
Sliding the semi-colon down to the next line suppresses the warning.
Code:
handler.c
---------
/* skip to next name */
- for (; isalpha(*curname); curname++);
+ for (; isalpha(*curname); curname++)
+ ;
+
if (!*curname)
return (0);
curname++; /* first char of new name */
Now to suppress another harmless warning in magic.c
Code:
magic.c
-------
break;
}
- if (failure || IdNum == -1) {
+ if (failure || (event_id)IdNum == -1) {
send_to_char(ch, "You failed!\r\n");
return;
}
Now just as in handler.c, the compiler complains about someone being too cute with for loops.
The errant indentation fix is of course optional.
Code:
modify.c
--------
/* Function that returns the number of pages in the string. */
static int count_pages(char *str, struct char_data *ch)
{
int pages;
- for (pages = 1; (str = next_page(str, ch)); pages++);
+ for (pages = 1; (str = next_page(str, ch)); pages++)
+ ;
- return (pages);
+ return (pages);
}
Finally, one more slightly less harmless issue in spec_procs.c
Code:
spec_procs.c
------------
#define SPLSKL(ch) (prac_types[prac_params[PRAC_TYPE][(int)GET_CLASS(ch)]])
void list_skills(struct char_data *ch)
{
const char *overflow = "\r\n**OVERFLOW**\r\n";
- int i, sortpos;
+ int i, sortpos, ret;
size_t len = 0, nlen;
char buf2[MAX_STRING_LENGTH];
len = snprintf(buf2, sizeof(buf2), "You have %d practice session%s remaining.\r\n"
"You know of the following %ss:\r\n", GET_PRACTICES(ch),
GET_PRACTICES(ch) == 1 ? "" : "s", SPLSKL(ch));
for (sortpos = 1; sortpos <= MAX_SKILLS; sortpos++) {
i = spell_sort_info[sortpos];
if (GET_LEVEL(ch) >= spell_info[i].min_level[(int) GET_CLASS(ch)]) {
- nlen = snprintf(buf2 + len, sizeof(buf2) - len, "%-20s %s\r\n", spell_info[i].name, how_good(GET_SKILL(ch, i)));
+ ret = snprintf(buf2 + len, sizeof(buf2) - len, "%-20s %s\r\n", spell_info[i].name, how_good(GET_SKILL(ch, i)));
- if (len + nlen >= sizeof(buf2) || nlen < 0)
+ if (len + nlen >= sizeof(buf2) || ret < 0)
break;
len += nlen;
}
}
if (len >= sizeof(buf2))
strcpy(buf2 + sizeof(buf2) - strlen(overflow) - 1, overflow); /* strcpy: OK */
page_string(ch->desc, buf2, TRUE);
}
