Hi,
turns out that there are a few more bad savestr calls even inside of pch.c.
Some of the code pathes are quite obvious that a NULL return value would
lead to a null pointer dereference, others would last longer before
dereferencing.
Carefully reviewing the savestr calls, the rule of thumb seems to be:
If savestr is used, immediately check out_of_mem, otherwise use xstrdup.
Theo pointed out that this one is very ugly (indeed):
+ if (!s)
+ s = Oops;
Yet it's current behavior of savestr. I want to remove that in another
commit. So for now, xstrdup behaves like savestr, calling fatal in case
of out of memory situation -- regardless of plan a or plan b.
Tobias
Index: patch.c
===
RCS file: /cvs/src/usr.bin/patch/patch.c,v
retrieving revision 1.51
diff -u -p -r1.51 patch.c
--- patch.c 26 Nov 2013 13:19:07 - 1.51
+++ patch.c 26 Nov 2014 11:46:20 -
@@ -213,7 +213,7 @@ main(int argc, char *argv[])
warn_on_invalid_line = true;
if (outname == NULL)
- outname = savestr(filearg[0]);
+ outname = xstrdup(filearg[0]);
/* for ed script just up and do it and exit */
if (diff_type == ED_DIFF) {
@@ -491,10 +491,10 @@ get_some_switches(void)
/* FALLTHROUGH */
case 'z':
/* must directly follow 'b' case for backwards compat */
- simple_backup_suffix = savestr(optarg);
+ simple_backup_suffix = xstrdup(optarg);
break;
case 'B':
- origprae = savestr(optarg);
+ origprae = xstrdup(optarg);
break;
case 'c':
diff_type = CONTEXT_DIFF;
@@ -532,7 +532,7 @@ get_some_switches(void)
case 'i':
if (++filec == MAXFILEC)
fatal(too many file arguments\n);
- filearg[filec] = savestr(optarg);
+ filearg[filec] = xstrdup(optarg);
break;
case 'l':
canonicalize = true;
@@ -544,7 +544,7 @@ get_some_switches(void)
noreverse = true;
break;
case 'o':
- outname = savestr(optarg);
+ outname = xstrdup(optarg);
break;
case 'p':
strippath = atoi(optarg);
@@ -588,12 +588,12 @@ get_some_switches(void)
Argv += optind;
if (Argc 0) {
- filearg[0] = savestr(*Argv++);
+ filearg[0] = xstrdup(*Argv++);
Argc--;
while (Argc 0) {
if (++filec == MAXFILEC)
fatal(too many file arguments\n);
- filearg[filec] = savestr(*Argv++);
+ filearg[filec] = xstrdup(*Argv++);
Argc--;
}
}
Index: pch.c
===
RCS file: /cvs/src/usr.bin/patch/pch.c,v
retrieving revision 1.46
diff -u -p -r1.46 pch.c
--- pch.c 26 Nov 2014 10:11:21 - 1.46
+++ pch.c 26 Nov 2014 11:46:21 -
@@ -215,14 +215,14 @@ there_is_another_patch(void)
while (filearg[0] == NULL) {
if (force || batch) {
say(No file to patch. Skipping...\n);
- filearg[0] = savestr(bestguess);
+ filearg[0] = xstrdup(bestguess);
skip_rest_of_patch = true;
return true;
}
ask(File to patch: );
if (*buf != '\n') {
free(bestguess);
- bestguess = savestr(buf);
+ bestguess = xstrdup(buf);
filearg[0] = fetchname(buf, exists, 0);
}
if (!exists) {
@@ -310,7 +310,7 @@ intuit_diff_type(void)
else if (strnEQ(s, Prereq:, 7)) {
for (t = s + 7; isspace((unsigned char)*t); t++)
;
- revision = savestr(t);
+ revision = xstrdup(t);
for (t = revision;
*t !isspace((unsigned char)*t); t++)
;
@@ -389,7 +389,7 @@ scan_exit:
free(bestguess);
bestguess = NULL;
if (filearg[0] != NULL)
- bestguess = savestr(filearg[0]);
+ bestguess = xstrdup(filearg[0]);
else if (!ok_to_create_file) {
/*
* We don't want to create a new file but we need a
@@ -1473,7
