Skip to content

Commit d74adbf

Browse files
committed
Fail rdump_set_oom_dump() loudly when the path copy fails
strdup() can fail right when callers reach for this API -- near the memory_limit. Previously its NULL return went unchecked while the override was still marked set, so the OOM auto-dump was silently disabled with no fallback to the rdump.oom_dump INI default. Now return false with an E_WARNING and leave the override cleared; return true on success. https://claude.ai/code/session_017skw8BsHkF8wur7Pf4G9W5
1 parent c6a7588 commit d74adbf

2 files changed

Lines changed: 19 additions & 3 deletions

File tree

README.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,9 @@ rdump_set_oom_dump("/var/log/oom-" . getmypid() . ".rdump");
124124
```
125125

126126
The runtime setting applies to the current request and takes precedence over
127-
the `rdump.oom_dump` default.
127+
the `rdump.oom_dump` default. It returns `true` on success, or `false` (with an
128+
`E_WARNING`) if the path could not be stored, leaving the previous default in
129+
effect rather than silently disabling the dump.
128130

129131
## Analyse with reli
130132

rdump.c

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -602,14 +602,16 @@ static void rdump_zend_error_cb(RDUMP_ERROR_CB_PARAMS)
602602
}
603603

604604
/* ------------------------------------------------------------------ */
605-
/* PHP function: void rdump_set_oom_dump(?string $path, bool $full=false)
605+
/* PHP function: bool rdump_set_oom_dump(?string $path, bool $full=false)
606606
*
607607
* Runtime toggle for the OOM auto-dump, overriding the INI default for
608608
* the rest of the current request:
609609
* - non-empty path -> enable, writing to that path (so callers can use
610610
* a per-request/per-pid filename the static INI cannot express);
611611
* - "" -> force-disable even if rdump.oom_dump is set;
612612
* - null -> clear the override and fall back to the INI.
613+
* Returns true on success, or false (with an E_WARNING) if the path could
614+
* not be stored, in which case the override is left cleared.
613615
*/
614616
/* ------------------------------------------------------------------ */
615617

@@ -633,10 +635,22 @@ PHP_FUNCTION(rdump_set_oom_dump)
633635
rdump_clear_runtime_oom_dump();
634636

635637
if (path != NULL) {
636-
RDUMP_G(oom_dump_runtime) = strdup(path);
638+
char *copy = strdup(path);
639+
if (copy == NULL) {
640+
/* strdup can fail exactly when callers reach for this API -- near
641+
* the memory_limit. Leave the override cleared (not half-set) so
642+
* resolution falls back to the rdump.oom_dump INI default rather
643+
* than silently disabling the auto-dump, and signal the failure. */
644+
php_error_docref(NULL, E_WARNING,
645+
"rdump_set_oom_dump: out of memory copying the path");
646+
RETURN_FALSE;
647+
}
648+
RDUMP_G(oom_dump_runtime) = copy;
637649
RDUMP_G(oom_dump_runtime_set) = 1;
638650
RDUMP_G(oom_dump_runtime_full) = full ? 1 : 0;
639651
}
652+
653+
RETURN_TRUE;
640654
}
641655

642656
/* ------------------------------------------------------------------ */

0 commit comments

Comments
 (0)