[PHP] Remove unnecessary zval initialization (#9600)

* [PHP] remove unneccesary zval

* fix memory leak, warning, Makefile

Makefile.am

@@ -840,6 +840,7 @@ php_EXTRA_DIST=                                                       \
   php/ext/google/protobuf/protobuf.c                                  \
   php/ext/google/protobuf/protobuf.h                                  \
   php/ext/google/protobuf/wkt.inc                                     \
+  php/ext/google/protobuf/tests/unnecessary_zval.phpt                 \
   php/generate_descriptor_protos.sh                                   \
   php/generate_test_protos.sh                                         \
   php/release.sh                                                      \

php/ext/google/protobuf/def.c

@@ -732,20 +732,16 @@ void DescriptorPool_CreateWithSymbolTable(zval *zv, upb_DefPool *symtab) {
 }
 
 upb_DefPool *DescriptorPool_GetSymbolTable() {
-  DescriptorPool *intern = GetPool(get_generated_pool());
-  return intern->symtab;
+  return get_global_symtab();
 }
 
-
 /*
  * DescriptorPool::getGeneratedPool()
  *
  * Returns the generated DescriptorPool.
  */
 PHP_METHOD(DescriptorPool, getGeneratedPool) {
-  zval ret;
-  ZVAL_COPY(&ret, get_generated_pool());
-  RETURN_COPY_VALUE(&ret);
+  DescriptorPool_CreateWithSymbolTable(return_value, get_global_symtab());
 }
 
 /*
@@ -880,14 +876,14 @@ static void add_name_mappings(const upb_FileDef *file) {
   }
 }
 
-static void add_descriptor(DescriptorPool *pool,
+static void add_descriptor(upb_DefPool *symtab,
                            const google_protobuf_FileDescriptorProto *file) {
   upb_StringView name = google_protobuf_FileDescriptorProto_name(file);
   upb_Status status;
   const upb_FileDef *file_def;
   upb_Status_Clear(&status);
 
-  if (upb_DefPool_FindFileByNameWithSize(pool->symtab, name.data, name.size)) {
+  if (upb_DefPool_FindFileByNameWithSize(symtab, name.data, name.size)) {
     // Already added.
     // TODO(teboring): Re-enable this warning when aggregate metadata is
     // deprecated.
@@ -902,10 +898,10 @@ static void add_descriptor(DescriptorPool *pool,
   // doesn't add it as a dependency even if the proto file actually does
   // depend on it.
   if (depends_on_descriptor(file)) {
-    google_protobuf_FileDescriptorProto_getmsgdef(pool->symtab);
+    google_protobuf_FileDescriptorProto_getmsgdef(symtab);
   }
 
-  file_def = upb_DefPool_AddFile(pool->symtab, file, &status);
+  file_def = upb_DefPool_AddFile(symtab, file, &status);
   CheckUpbStatus(&status, "Unable to load descriptor");
   add_name_mappings(file_def);
 }
@@ -915,7 +911,7 @@ static void add_descriptor(DescriptorPool *pool,
  *
  * Adds the given descriptor data to this DescriptorPool.
  */
-static void add_descriptor_set(DescriptorPool *pool, const char *data,
+static void add_descriptor_set(upb_DefPool *symtab, const char *data,
                                int data_len, upb_Arena *arena) {
   size_t i, n;
   google_protobuf_FileDescriptorSet *set;
@@ -932,13 +928,12 @@ static void add_descriptor_set(DescriptorPool *pool, const char *data,
 
   for (i = 0; i < n; i++) {
     const google_protobuf_FileDescriptorProto* file = files[i];
-    add_descriptor(pool, file);
+    add_descriptor(symtab, file);
   }
 }
 
 bool DescriptorPool_HasFile(const char *filename) {
-  DescriptorPool *intern = GetPool(get_generated_pool());
-  return upb_DefPool_FindFileByName(intern->symtab, filename) != NULL;
+  return upb_DefPool_FindFileByName(get_global_symtab(), filename) != NULL;
 }
 
 void DescriptorPool_AddDescriptor(const char *filename, const char *data,
@@ -952,7 +947,7 @@ void DescriptorPool_AddDescriptor(const char *filename, const char *data,
     return;
   }
 
-  add_descriptor(GetPool(get_generated_pool()), file);
+  add_descriptor(get_global_symtab(), file);
   upb_Arena_Free(arena);
 }
 
@@ -974,7 +969,7 @@ PHP_METHOD(DescriptorPool, internalAddGeneratedFile) {
   }
 
   arena = upb_Arena_New();
-  add_descriptor_set(intern, data, data_len, arena);
+  add_descriptor_set(intern->symtab, data, data_len, arena);
   upb_Arena_Free(arena);
 }
 
@@ -1015,7 +1010,7 @@ zend_class_entry *InternalDescriptorPool_class_entry;
  * instance.
  */
 PHP_METHOD(InternalDescriptorPool, getGeneratedPool) {
-  RETURN_COPY(get_generated_pool());
+  DescriptorPool_CreateWithSymbolTable(return_value, get_global_symtab());
 }
 
 static zend_function_entry InternalDescriptorPool_methods[] = {

php/ext/google/protobuf/protobuf.c

@@ -56,12 +56,6 @@ ZEND_BEGIN_MODULE_GLOBALS(protobuf)
   // Set by the user to make the descriptor pool persist between requests.
   zend_bool keep_descriptor_pool_after_request;
 
-  // Currently we make the generated pool a "global", which means that if a user
-  // does explicitly create threads within their request, the other threads will
-  // get different results from DescriptorPool::getGeneratedPool(). We require
-  // that all descriptors are loaded from the main thread.
-  zval generated_pool;
-
   // A upb_DefPool that we are saving for the next request so that we don't have
   // to rebuild it from scratch. When keep_descriptor_pool_after_request==true,
   // we steal the upb_DefPool from the global DescriptorPool object just before
@@ -91,8 +85,8 @@ void free_protobuf_globals(zend_protobuf_globals *globals) {
 
 ZEND_DECLARE_MODULE_GLOBALS(protobuf)
 
-const zval *get_generated_pool() {
-  return &PROTOBUF_G(generated_pool);
+upb_DefPool *get_global_symtab() {
+  return PROTOBUF_G(global_symtab);
 }
 
 // This is a PHP extension (not a Zend extension). What follows is a summary of
@@ -159,7 +153,6 @@ static PHP_GSHUTDOWN_FUNCTION(protobuf) {
 }
 
 static PHP_GINIT_FUNCTION(protobuf) {
-  ZVAL_NULL(&protobuf_globals->generated_pool);
   protobuf_globals->global_symtab = NULL;
 }
 
@@ -177,7 +170,6 @@ static PHP_RINIT_FUNCTION(protobuf) {
     zend_hash_init(&PROTOBUF_G(name_msg_cache), 64, NULL, NULL, 0);
     zend_hash_init(&PROTOBUF_G(name_enum_cache), 64, NULL, NULL, 0);
   }
-  DescriptorPool_CreateWithSymbolTable(&PROTOBUF_G(generated_pool), symtab);
 
   zend_hash_init(&PROTOBUF_G(object_cache), 64, NULL, NULL, 0);
   zend_hash_init(&PROTOBUF_G(descriptors), 64, NULL, ZVAL_PTR_DTOR, 0);
@@ -196,7 +188,6 @@ static PHP_RSHUTDOWN_FUNCTION(protobuf) {
     free_protobuf_globals(ZEND_MODULE_GLOBALS_BULK(protobuf));
   }
 
-  zval_dtor(&PROTOBUF_G(generated_pool));
   zend_hash_destroy(&PROTOBUF_G(object_cache));
   zend_hash_destroy(&PROTOBUF_G(descriptors));
 

php/ext/google/protobuf/protobuf.h

@@ -36,7 +36,7 @@
 
 #include "php-upb.h"
 
-const zval *get_generated_pool();
+upb_DefPool *get_global_symtab();
 
 #if PHP_VERSION_ID < 70300
 #define GC_ADDREF(h) ++GC_REFCOUNT(h)

php/ext/google/protobuf/tests/unnecessary_zval.phpt

@@ -0,0 +1,9 @@
+--TEST--
+unnecessary zval
+--FILE--
+<?php
+var_dump(new \stdClass());
+?>
+--EXPECT--
+object(stdClass)#1 (0) {
+}

php/tests/compile_extension.sh

@@ -34,4 +34,5 @@ if [[ ! -f BUILD_STAMP ]] || [[ "$(cat BUILD_STAMP)" != "$FINGERPRINT" ]]; then
 fi
 
 make
+TEST_PHP_ARGS="-q" make test
 popd > /dev/null