Skip to content

Commit 0366899

Browse files
GirgiasGirgias
authored
ext/phar: refactor phar_get_archive() to return phar_archive_data* (#21858)
Instead of using an out param.
1 parent 1095cf5 commit 0366899

8 files changed

Lines changed: 104 additions & 122 deletions

File tree

‎ext/phar/dirstream.c‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,6 @@ static php_stream *phar_make_dirstream(const char *dir, size_t dirlen, const Has
248248
php_stream *phar_wrapper_open_dir(php_stream_wrapper *wrapper, const char *path, const char *mode, int options, zend_string **opened_path, php_stream_context *context STREAMS_DC) /* {{{ */
249249
{
250250
char *error;
251-
phar_archive_data *phar;
252251

253252
php_url *resource = phar_parse_url(wrapper, path, mode, options);
254253
if (!resource) {
@@ -276,7 +275,8 @@ php_stream *phar_wrapper_open_dir(php_stream_wrapper *wrapper, const char *path,
276275

277276
phar_request_initialize();
278277

279-
if (FAILURE == phar_get_archive(&phar, ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), NULL, 0, &error)) {
278+
const phar_archive_data *phar = phar_get_archive(ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), NULL, 0, &error);
279+
if (!phar) {
280280
if (error) {
281281
php_stream_wrapper_log_error(wrapper, options, "%s", error);
282282
efree(error);
@@ -340,7 +340,6 @@ php_stream *phar_wrapper_open_dir(php_stream_wrapper *wrapper, const char *path,
340340
int phar_wrapper_mkdir(php_stream_wrapper *wrapper, const char *url_from, int mode, int options, php_stream_context *context) /* {{{ */
341341
{
342342
phar_entry_info entry;
343-
phar_archive_data *phar = NULL;
344343
char *error;
345344
php_url *resource = NULL;
346345

@@ -351,9 +350,7 @@ int phar_wrapper_mkdir(php_stream_wrapper *wrapper, const char *url_from, int mo
351350
return 0;
352351
}
353352

354-
if (FAILURE == phar_get_archive(&phar, ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL)) {
355-
phar = NULL;
356-
}
353+
phar_archive_data *phar = phar_get_archive(ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL);
357354

358355
zend_string_release_ex(arch, false);
359356

@@ -379,7 +376,8 @@ int phar_wrapper_mkdir(php_stream_wrapper *wrapper, const char *url_from, int mo
379376
return 0;
380377
}
381378

382-
if (FAILURE == phar_get_archive(&phar, ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), NULL, 0, &error)) {
379+
phar = phar_get_archive(ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), NULL, 0, &error);
380+
if (!phar) {
383381
php_stream_wrapper_log_error(wrapper, options, "phar error: cannot create directory \"%s\" in phar \"%s\", error retrieving phar information: %s", ZSTR_VAL(resource->path) + 1, ZSTR_VAL(resource->host), error);
384382
efree(error);
385383
php_url_free(resource);
@@ -467,7 +465,6 @@ int phar_wrapper_mkdir(php_stream_wrapper *wrapper, const char *url_from, int mo
467465
*/
468466
int phar_wrapper_rmdir(php_stream_wrapper *wrapper, const char *url, int options, php_stream_context *context) /* {{{ */
469467
{
470-
phar_archive_data *phar = NULL;
471468
char *error;
472469

473470
/* pre-readonly check, we need to know if this is a data phar */
@@ -477,9 +474,7 @@ int phar_wrapper_rmdir(php_stream_wrapper *wrapper, const char *url, int options
477474
return 0;
478475
}
479476

480-
if (FAILURE == phar_get_archive(&phar, ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL)) {
481-
phar = NULL;
482-
}
477+
phar_archive_data *phar = phar_get_archive(ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL);
483478

484479
zend_string_release_ex(arch, false);
485480

@@ -506,7 +501,8 @@ int phar_wrapper_rmdir(php_stream_wrapper *wrapper, const char *url, int options
506501
return 0;
507502
}
508503

509-
if (FAILURE == phar_get_archive(&phar, ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), NULL, 0, &error)) {
504+
phar = phar_get_archive(ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), NULL, 0, &error);
505+
if (!phar) {
510506
php_stream_wrapper_log_error(wrapper, options, "phar error: cannot remove directory \"%s\" in phar \"%s\", error retrieving phar information: %s", ZSTR_VAL(resource->path)+1, ZSTR_VAL(resource->host), error);
511507
efree(error);
512508
php_url_free(resource);

‎ext/phar/func_interceptors.c‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ static zend_string* phar_get_name_for_relative_paths(zend_string *filename, bool
9898

9999
/* fopen within phar, if :// is not in the url, then prepend phar://<archive>/ */
100100
/* retrieving a file defaults to within the current directory, so use this if possible */
101-
phar_archive_data *phar;
102-
if (FAILURE == phar_get_archive(&phar, ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL)) {
101+
phar_archive_data *phar = phar_get_archive(ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL);
102+
if (!phar) {
103103
zend_string_release_ex(arch, false);
104104
return NULL;
105105
}
@@ -492,9 +492,9 @@ static void phar_file_stat(const char *filename, size_t filename_length, int typ
492492
zend_string *arch = phar_split_fname(ZSTR_VAL(fname), ZSTR_LEN(fname), NULL, 2, 0);
493493
if (arch) {
494494
/* fopen within phar, if :// is not in the url, then prepend phar://<archive>/ */
495-
zend_result has_archive = phar_get_archive(&phar, ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL);
495+
phar = phar_get_archive(ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL);
496496
zend_string_release_ex(arch, false);
497-
if (FAILURE == has_archive) {
497+
if (!phar) {
498498
goto skip_phar;
499499
}
500500
splitted:;
@@ -727,13 +727,13 @@ PHP_FUNCTION(phar_is_file) /* {{{ */
727727

728728
zend_string *arch = phar_split_fname(ZSTR_VAL(fname), ZSTR_LEN(fname), NULL, 2, 0);
729729
if (arch) {
730-
phar_archive_data *phar;
730+
;
731731

732732
/* fopen within phar, if :// is not in the url, then prepend phar://<archive>/ */
733733
/* retrieving a file within the current directory, so use this if possible */
734-
zend_result has_archive = phar_get_archive(&phar, ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL);
734+
phar_archive_data *phar = phar_get_archive(ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL);
735735
zend_string_release_ex(arch, false);
736-
if (has_archive == SUCCESS) {
736+
if (phar) {
737737
phar_entry_info *etemp;
738738

739739
zend_string *entry = phar_fix_filepath(filename, filename_len, true);
@@ -784,13 +784,11 @@ PHP_FUNCTION(phar_is_link) /* {{{ */
784784

785785
zend_string *arch = phar_split_fname(ZSTR_VAL(fname), ZSTR_LEN(fname), NULL, 2, 0);
786786
if (arch) {
787-
phar_archive_data *phar;
788-
789787
/* fopen within phar, if :// is not in the url, then prepend phar://<archive>/ */
790788
/* retrieving a file within the current directory, so use this if possible */
791-
zend_result has_archive = phar_get_archive(&phar, ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL);
789+
phar_archive_data *phar = phar_get_archive(ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL);
792790
zend_string_release_ex(arch, false);
793-
if (has_archive == SUCCESS) {
791+
if (phar) {
794792
phar_entry_info *etemp;
795793

796794
zend_string *entry = phar_fix_filepath(filename, filename_len, true);

‎ext/phar/phar.c‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,6 @@ ZEND_ATTRIBUTE_NONNULL void phar_entry_remove(phar_entry_data *idata, char **err
486486
*/
487487
static zend_result phar_open_parsed_phar(char *fname, size_t fname_len, char *alias, size_t alias_len, bool is_data, uint32_t options, phar_archive_data** pphar, char **error) /* {{{ */
488488
{
489-
phar_archive_data *phar;
490489
#ifdef PHP_WIN32
491490
char *save_fname;
492491
ALLOCA_FLAG(fname_use_heap)
@@ -504,12 +503,12 @@ static zend_result phar_open_parsed_phar(char *fname, size_t fname_len, char *al
504503
phar_unixify_path_separators(fname, fname_len);
505504
}
506505
#endif
507-
zend_result archive_retrieved = phar_get_archive(&phar, fname, fname_len, alias, alias_len, error);
506+
phar_archive_data *phar = phar_get_archive(fname, fname_len, alias, alias_len, error);
508507
/* logic is as follows:
509508
- If no alias was passed in, then it can match either and be valid
510509
- If an explicit alias was requested, ensure the filename passed in matches the phar's filename.
511510
*/
512-
bool process_phar = SUCCESS == archive_retrieved && (!alias || zend_string_equals_cstr(phar->fname, fname, fname_len));
511+
bool process_phar = phar && (!alias || zend_string_equals_cstr(phar->fname, fname, fname_len));
513512
#ifdef PHP_WIN32
514513
if (fname != save_fname) {
515514
free_alloca(fname, fname_use_heap);

‎ext/phar/phar_internal.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(1, 6, 7) zend_result phar_open_or_create_filename(ze
411411
ZEND_ATTRIBUTE_NONNULL_ARGS(1, 6, 7) zend_result phar_create_or_parse_filename(zend_string *fname, char *alias, size_t alias_len, bool is_data, uint32_t options, phar_archive_data** pphar, char **error);
412412
ZEND_ATTRIBUTE_NONNULL_ARGS(3) zend_result phar_open_executed_filename(char *alias, size_t alias_len, char **error);
413413
zend_result phar_free_alias(const phar_archive_data *phar);
414-
zend_result phar_get_archive(phar_archive_data **archive, const char *fname, size_t fname_len, const char *alias, size_t alias_len, char **error);
414+
phar_archive_data* phar_get_archive(const char *fname, size_t fname_len, const char *alias, size_t alias_len, char **error);
415415
zend_result phar_verify_signature(php_stream *fp, size_t end_of_phar, uint32_t sig_type, const char *sig, size_t sig_len, const char *fname, char **signature, size_t *signature_len, char **error);
416416
ZEND_ATTRIBUTE_NONNULL zend_string* phar_create_signature(phar_archive_data *phar, php_stream *fp, char **error);
417417

‎ext/phar/phar_object.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -752,8 +752,8 @@ PHP_METHOD(Phar, webPhar)
752752
entry_len = sizeof("/index.php")-1;
753753
}
754754

755-
if (FAILURE == phar_get_archive(&phar, fname, fname_len, NULL, 0, NULL) ||
756-
(info = phar_get_entry_info(phar, entry, entry_len, NULL, false)) == NULL) {
755+
phar = phar_get_archive(fname, fname_len, NULL, 0, NULL);
756+
if (!phar || (info = phar_get_entry_info(phar, entry, entry_len, NULL, false)) == NULL) {
757757
phar_do_404(phar, fname, fname_len, f404);
758758
} else {
759759
char *tmp = NULL, sa = '\0';
@@ -793,8 +793,8 @@ PHP_METHOD(Phar, webPhar)
793793
goto cleanup_skip_entry;
794794
}
795795

796-
if (FAILURE == phar_get_archive(&phar, fname, fname_len, NULL, 0, NULL) ||
797-
(info = phar_get_entry_info(phar, entry, entry_len, NULL, false)) == NULL) {
796+
phar = phar_get_archive(fname, fname_len, NULL, 0, NULL);
797+
if (!phar || (info = phar_get_entry_info(phar, entry, entry_len, NULL, false)) == NULL) {
798798
phar_do_404(phar, fname, fname_len, f404);
799799
goto cleanup;
800800
}

‎ext/phar/stream.c‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,8 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha
230230
} else {
231231
if (!*internal_file && (options & STREAM_OPEN_FOR_INCLUDE)) {
232232
/* retrieve the stub */
233-
if (FAILURE == phar_get_archive(&phar, ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), NULL, 0, NULL)) {
233+
phar = phar_get_archive(ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), NULL, 0, NULL);
234+
if (!phar) {
234235
php_stream_wrapper_log_error(wrapper, options, "file %s is not a valid phar archive", ZSTR_VAL(resource->host));
235236
efree(internal_file);
236237
php_url_free(resource);
@@ -554,7 +555,6 @@ static int phar_wrapper_stat(php_stream_wrapper *wrapper, const char *url, int f
554555
php_stream_statbuf *ssb, php_stream_context *context) /* {{{ */
555556
{
556557
char *internal_file;
557-
phar_archive_data *phar;
558558
size_t internal_file_len;
559559

560560
php_url *resource = phar_parse_url(wrapper, url, "r", flags|PHP_STREAM_URL_STAT_QUIET);
@@ -577,7 +577,8 @@ static int phar_wrapper_stat(php_stream_wrapper *wrapper, const char *url, int f
577577

578578
internal_file = ZSTR_VAL(resource->path) + 1; /* strip leading "/" */
579579
/* find the phar in our trusty global hash indexed by alias (host of phar://blah.phar/file.whatever) */
580-
if (FAILURE == phar_get_archive(&phar, ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), NULL, 0, NULL)) {
580+
phar_archive_data *phar = phar_get_archive(ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), NULL, 0, NULL);
581+
if (!phar) {
581582
php_url_free(resource);
582583
return FAILURE;
583584
}
@@ -725,7 +726,6 @@ static int phar_wrapper_unlink(php_stream_wrapper *wrapper, const char *url, int
725726
static int phar_wrapper_rename(php_stream_wrapper *wrapper, const char *url_from, const char *url_to, int options, php_stream_context *context) /* {{{ */
726727
{
727728
char *error;
728-
phar_archive_data *phar, *pfrom, *pto;
729729
bool is_dir = false;
730730
bool is_modified = false;
731731

@@ -736,9 +736,8 @@ static int phar_wrapper_rename(php_stream_wrapper *wrapper, const char *url_from
736736
php_error_docref(NULL, E_WARNING, "phar error: cannot rename \"%s\" to \"%s\": invalid or non-writable url \"%s\"", url_from, url_to, url_from);
737737
return 0;
738738
}
739-
if (SUCCESS != phar_get_archive(&pfrom, ZSTR_VAL(resource_from->host), ZSTR_LEN(resource_from->host), NULL, 0, NULL)) {
740-
pfrom = NULL;
741-
}
739+
740+
phar_archive_data *pfrom = phar_get_archive(ZSTR_VAL(resource_from->host), ZSTR_LEN(resource_from->host), NULL, 0, NULL);
742741
if (PHAR_G(readonly) && (!pfrom || !pfrom->is_data)) {
743742
php_url_free(resource_from);
744743
php_error_docref(NULL, E_WARNING, "phar error: Write operations disabled by the php.ini setting phar.readonly");
@@ -751,9 +750,8 @@ static int phar_wrapper_rename(php_stream_wrapper *wrapper, const char *url_from
751750
php_error_docref(NULL, E_WARNING, "phar error: cannot rename \"%s\" to \"%s\": invalid or non-writable url \"%s\"", url_from, url_to, url_to);
752751
return 0;
753752
}
754-
if (SUCCESS != phar_get_archive(&pto, ZSTR_VAL(resource_to->host), ZSTR_LEN(resource_to->host), NULL, 0, NULL)) {
755-
pto = NULL;
756-
}
753+
754+
phar_archive_data *pto = phar_get_archive(ZSTR_VAL(resource_to->host), ZSTR_LEN(resource_to->host), NULL, 0, NULL);
757755
if (PHAR_G(readonly) && (!pto || !pto->is_data)) {
758756
php_url_free(resource_from);
759757
php_url_free(resource_to);
@@ -797,7 +795,8 @@ static int phar_wrapper_rename(php_stream_wrapper *wrapper, const char *url_from
797795
return 0;
798796
}
799797

800-
if (SUCCESS != phar_get_archive(&phar, ZSTR_VAL(resource_from->host), ZSTR_LEN(resource_from->host), NULL, 0, &error)) {
798+
phar_archive_data *phar = phar_get_archive(ZSTR_VAL(resource_from->host), ZSTR_LEN(resource_from->host), NULL, 0, &error);
799+
if (!phar) {
801800
php_url_free(resource_from);
802801
php_url_free(resource_to);
803802
php_error_docref(NULL, E_WARNING, "phar error: cannot rename \"%s\" to \"%s\": %s", url_from, url_to, error);

0 commit comments

Comments
 (0)