Skip to content

Commit bcb54e5

Browse files
LibretroAdminLibretroAdmin
committed
tasks/task_audio_mixer: drop needless strdup+free of basename in 10 upload handlers
task_audio_mixer_handle_upload_* functions (10 variants: ogg, ogg_and_play, wav, wav_and_play, flac, flac_and_play, mp3, mp3_and_play, mod, mod_and_play) all shared the same pattern: params.basename = (img->path && *img->path) ? strdup(path_basename_nocompression(img->path)) : NULL; audio_driver_mixer_add_stream(&params); if (img->path) free(img->path); if (params.basename != NULL) free(params.basename); free(img); free(user_data); The strdup + matching free form a redundant round-trip: 1. path_basename_nocompression(img->path) returns a pointer INTO img->path (the basename portion within the full path string). No allocation - it's just a pointer walk that finds the last '/' and returns what follows. 2. strdup copies that into a fresh heap buffer B. 3. audio_driver_mixer_add_stream is called synchronously. Its only use of params->basename is at audio_driver.c:1508 audio_driver_st.mixer_streams[free_slot].name = (params->basename && *params->basename) ? strdup(params->basename) : NULL; which immediately strdups it into its own owned buffer C for long-term storage in mixer_streams[slot].name. 4. add_stream returns. 5. free(B) - releases the temporary we just made. 6. free(img->path) - releases the original string (which the path_basename_nocompression pointer aliased into). So B existed purely to bridge the basename's content from img->path (alive through step 6) into mixer_streams[slot].name (owned by add_stream via its own strdup at step 3). We can skip step 2 and step 5 entirely - pass the aliased pointer directly into params.basename. add_stream's strdup at step 3 captures the content before step 6 frees the backing string, so the aliased-pointer lifetime is valid for its one and only read. params.basename = (img->path && *img->path) ? (char*)path_basename_nocompression(img->path) : NULL; audio_driver_mixer_add_stream(&params); if (img->path) free(img->path); free(img); free(user_data); The cast to (char*) is because path_basename_nocompression returns const char* but the params.basename field is non-const char*. The non-const is vestigial from when the field was owning - add_stream never writes through it, only reads it once before strdup'ing. === Savings === Per audio mixer upload: 1 strdup + 1 free eliminated. Across 10 upload handlers (ogg/wav/flac/mp3/mod x and_play variants), 20 operations removed, -20 lines of code. The strdup overhead is O(basename_length) - small per call, but this fires on every cheevo unlock sound, menu BGM load, configurable hint sound, etc. On memory-constrained embedded targets (Vita, 3DS, classic consoles) every saved malloc helps with heap fragmentation. === Correctness analysis === Traced the full img pointer lifetime for every handler: - task_data is (nbio_buf_t*)task_data, created by the NBIO file-read task. The handler takes ownership via task->handler's task_data transfer. - img->path is a heap string owned by img. - path_basename_nocompression(img->path) returns a pointer that shares lifetime with img->path. - audio_driver_mixer_add_stream is synchronous - no reentrant task dispatch, no storage of params->basename beyond the strdup at line 1508. - After add_stream returns, img->path is no longer needed by anyone downstream. The handler free()s it, then img, then user_data. No aliasing hazard, no UAF, no leak. This mirrors the pattern already used by e.g. params.buf (= img->buf, not strdup'd), params.bufsize, and the other params fields. Verified the full chain in audio_driver.c: add_stream reads params->basename at line 1508 exactly once, strdups it, and never looks at the pointer again. All return-false paths above line 1508 never touch basename, so failure modes also don't create leaks. === Swept-clean in the same pass === Looked for the same pattern elsewhere: - core_backup.c:381 'strdup(path_basename(backup_path))' is NOT a candidate - subsequent path_remove_extension() modifies the string in place, so we need our own mutable copy. Correct as-is. - No other strdup(path_basename(...)) patterns in the RetroArch-owned code (deps/ and libretro-common/ not scanned, those have their own ownership conventions). Scope: local to task_audio_mixer.c. No API changes, no header changes. audio_driver_mixer_add_stream's contract on params->basename is unchanged (it still only reads it once via strdup). Thread-safety: all handlers run on the main task dispatch thread. No concurrency concerns.
1 parent 60bda54 commit bcb54e5

1 file changed

Lines changed: 10 additions & 30 deletions

File tree

‎tasks/task_audio_mixer.c‎

Lines changed: 10 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -115,14 +115,12 @@ static void task_audio_mixer_handle_upload_ogg(retro_task_t *task,
115115
params.buf = img->buf;
116116
params.bufsize = img->bufsize;
117117
params.cb = NULL;
118-
params.basename = (img->path && *img->path) ? strdup(path_basename_nocompression(img->path)) : NULL;
118+
params.basename = (img->path && *img->path) ? (char*)path_basename_nocompression(img->path) : NULL;
119119

120120
audio_driver_mixer_add_stream(&params);
121121

122122
if (img->path)
123123
free(img->path);
124-
if (params.basename != NULL)
125-
free(params.basename);
126124
free(img);
127125
free(user_data);
128126
}
@@ -146,14 +144,12 @@ static void task_audio_mixer_handle_upload_ogg_and_play(retro_task_t *task,
146144
params.buf = img->buf;
147145
params.bufsize = img->bufsize;
148146
params.cb = NULL;
149-
params.basename = (img->path && *img->path) ? strdup(path_basename_nocompression(img->path)) : NULL;
147+
params.basename = (img->path && *img->path) ? (char*)path_basename_nocompression(img->path) : NULL;
150148

151149
audio_driver_mixer_add_stream(&params);
152150

153151
if (img->path)
154152
free(img->path);
155-
if (params.basename != NULL)
156-
free(params.basename);
157153
free(img);
158154
free(user_data);
159155
}
@@ -177,14 +173,12 @@ static void task_audio_mixer_handle_upload_flac(retro_task_t *task,
177173
params.buf = img->buf;
178174
params.bufsize = img->bufsize;
179175
params.cb = NULL;
180-
params.basename = (img->path && *img->path) ? strdup(path_basename_nocompression(img->path)) : NULL;
176+
params.basename = (img->path && *img->path) ? (char*)path_basename_nocompression(img->path) : NULL;
181177

182178
audio_driver_mixer_add_stream(&params);
183179

184180
if (img->path)
185181
free(img->path);
186-
if (params.basename != NULL)
187-
free(params.basename);
188182
free(img);
189183
free(user_data);
190184
}
@@ -208,14 +202,12 @@ static void task_audio_mixer_handle_upload_flac_and_play(retro_task_t *task,
208202
params.buf = img->buf;
209203
params.bufsize = img->bufsize;
210204
params.cb = NULL;
211-
params.basename = (img->path && *img->path) ? strdup(path_basename_nocompression(img->path)) : NULL;
205+
params.basename = (img->path && *img->path) ? (char*)path_basename_nocompression(img->path) : NULL;
212206

213207
audio_driver_mixer_add_stream(&params);
214208

215209
if (img->path)
216210
free(img->path);
217-
if (params.basename != NULL)
218-
free(params.basename);
219211
free(img);
220212
free(user_data);
221213
}
@@ -239,14 +231,12 @@ static void task_audio_mixer_handle_upload_mp3(retro_task_t *task,
239231
params.buf = img->buf;
240232
params.bufsize = img->bufsize;
241233
params.cb = NULL;
242-
params.basename = (img->path && *img->path) ? strdup(path_basename_nocompression(img->path)) : NULL;
234+
params.basename = (img->path && *img->path) ? (char*)path_basename_nocompression(img->path) : NULL;
243235

244236
audio_driver_mixer_add_stream(&params);
245237

246238
if (img->path)
247239
free(img->path);
248-
if (params.basename != NULL)
249-
free(params.basename);
250240
free(img);
251241
free(user_data);
252242
}
@@ -270,14 +260,12 @@ static void task_audio_mixer_handle_upload_mp3_and_play(retro_task_t *task,
270260
params.buf = img->buf;
271261
params.bufsize = img->bufsize;
272262
params.cb = NULL;
273-
params.basename = (img->path && *img->path) ? strdup(path_basename_nocompression(img->path)) : NULL;
263+
params.basename = (img->path && *img->path) ? (char*)path_basename_nocompression(img->path) : NULL;
274264

275265
audio_driver_mixer_add_stream(&params);
276266

277267
if (img->path)
278268
free(img->path);
279-
if (params.basename != NULL)
280-
free(params.basename);
281269
free(img);
282270
free(user_data);
283271
}
@@ -301,14 +289,12 @@ static void task_audio_mixer_handle_upload_mod(retro_task_t *task,
301289
params.buf = img->buf;
302290
params.bufsize = img->bufsize;
303291
params.cb = NULL;
304-
params.basename = (img->path && *img->path) ? strdup(path_basename_nocompression(img->path)) : NULL;
292+
params.basename = (img->path && *img->path) ? (char*)path_basename_nocompression(img->path) : NULL;
305293

306294
audio_driver_mixer_add_stream(&params);
307295

308296
if (img->path)
309297
free(img->path);
310-
if (params.basename != NULL)
311-
free(params.basename);
312298
free(img);
313299
free(user_data);
314300
}
@@ -332,14 +318,12 @@ static void task_audio_mixer_handle_upload_mod_and_play(retro_task_t *task,
332318
params.buf = img->buf;
333319
params.bufsize = img->bufsize;
334320
params.cb = NULL;
335-
params.basename = (img->path && *img->path) ? strdup(path_basename_nocompression(img->path)) : NULL;
321+
params.basename = (img->path && *img->path) ? (char*)path_basename_nocompression(img->path) : NULL;
336322

337323
audio_driver_mixer_add_stream(&params);
338324

339325
if (img->path)
340326
free(img->path);
341-
if (params.basename != NULL)
342-
free(params.basename);
343327
free(img);
344328
free(user_data);
345329
}
@@ -364,14 +348,12 @@ static void task_audio_mixer_handle_upload_wav(retro_task_t *task,
364348
params.buf = img->buf;
365349
params.bufsize = img->bufsize;
366350
params.cb = NULL;
367-
params.basename = (img->path && *img->path) ? strdup(path_basename_nocompression(img->path)) : NULL;
351+
params.basename = (img->path && *img->path) ? (char*)path_basename_nocompression(img->path) : NULL;
368352

369353
audio_driver_mixer_add_stream(&params);
370354

371355
if (img->path)
372356
free(img->path);
373-
if (params.basename != NULL)
374-
free(params.basename);
375357
free(img);
376358
free(user_data);
377359
}
@@ -395,14 +377,12 @@ static void task_audio_mixer_handle_upload_wav_and_play(retro_task_t *task,
395377
params.buf = img->buf;
396378
params.bufsize = img->bufsize;
397379
params.cb = NULL;
398-
params.basename = (img->path && *img->path) ? strdup(path_basename_nocompression(img->path)) : NULL;
380+
params.basename = (img->path && *img->path) ? (char*)path_basename_nocompression(img->path) : NULL;
399381

400382
audio_driver_mixer_add_stream(&params);
401383

402384
if (img->path)
403385
free(img->path);
404-
if (params.basename != NULL)
405-
free(params.basename);
406386
free(img);
407387
free(user_data);
408388
}

0 commit comments

Comments
 (0)