From patchwork Thu Aug 31 19:37:44 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 22489 Received: (qmail 114689 invoked by alias); 31 Aug 2017 19:37:58 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 114677 invoked by uid 89); 31 Aug 2017 19:37:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3 autolearn=ham version=3.3.2 spammy=sk:environ, (unknown), Our X-HELO: sessmg22.ericsson.net Received: from sessmg22.ericsson.net (HELO sessmg22.ericsson.net) (193.180.251.58) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 31 Aug 2017 19:37:54 +0000 Received: from ESESSHC018.ericsson.se (Unknown_Domain [153.88.183.72]) by sessmg22.ericsson.net (Symantec Mail Security) with SMTP id D4.B8.20899.F8568A95; Thu, 31 Aug 2017 21:37:51 +0200 (CEST) Received: from EUR01-HE1-obe.outbound.protection.outlook.com (153.88.183.145) by oa.msg.ericsson.com (153.88.183.72) with Microsoft SMTP Server (TLS) id 14.3.352.0; Thu, 31 Aug 2017 21:37:50 +0200 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=simon.marchi@ericsson.com; Received: from [192.168.0.102] (80.216.43.226) by DBXPR07MB318.eurprd07.prod.outlook.com (2a01:111:e400:941d::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.35.3; Thu, 31 Aug 2017 19:37:48 +0000 Subject: Re: [PATCH v4] Implement the ability to set/unset environment variables to GDBserver when starting the inferior To: Sergio Durigan Junior , GDB Patches CC: Pedro Alves , Simon Marchi References: <20170629194106.23070-1-sergiodj@redhat.com> <20170828231315.14952-1-sergiodj@redhat.com> From: Simon Marchi Message-ID: <28c9905c-b845-a1e7-368e-fc4631e59218@ericsson.com> Date: Thu, 31 Aug 2017 21:37:44 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170828231315.14952-1-sergiodj@redhat.com> X-ClientProxiedBy: DB6PR07CA0191.eurprd07.prod.outlook.com (2603:10a6:6:42::21) To DBXPR07MB318.eurprd07.prod.outlook.com (2a01:111:e400:941d::12) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 639e4386-17a4-48f1-6eb5-08d4f0a7c306 X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(300000500095)(300135000095)(300000501095)(300135300095)(300000502095)(300135100095)(22001)(2017030254152)(300000503095)(300135400095)(201703131423075)(201703031133081)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095); SRVR:DBXPR07MB318; X-Microsoft-Exchange-Diagnostics: 1; DBXPR07MB318; 3:3etD1qMA/O2bmGTyH7ZAdEOGPzJncZjPfUYjm82pOyscDu5X7Ba9efKxzc/ctYQMIQHaAGkhYTIBHVYdX7FyIucERFGyEbAME4z3KY5qscWSoRlKtdt29ZVw73XaHCCUGwEcPqdgpEiHUNTjdKT21kgqiUpt2IClX2NatVGO4z8f6WtJcJ3BID7SS1Nw5xoG4BP+yNfBrbwkiG73aNObklTrN/3G6WaJy8HOm99DOqsZTRGAp+AV0/uVKb4BycsT; 25:lCLtQ760oJoMAZwzYcmYkSC2oNupHDh39qp6/ffjTm1i1Sx4S7XEA3DLTMBVPZKDWxZ3jKjQ0jpyWV/YuqFWw7Ohakpp3y4gAEDrGuG/SYaxoUUXHnQEDM9zlHChjb57gbl80WeL1uu3tLnpwcaWHutCXLZLFtW3g8p4wdVtrV/OL0wY/TvQ5TdiXp4aUnYSYWxo59smNNhbfapd7VPKrKsL6l1JcLj5G1GvdsQR7E2g7kYDmooAyk6lEDl/NtyxC+46c1Fz4BNHuMBLcDzSuF7x0PKcx4VCbMg60GztJVbrc97KiU/1kSpfFzT9QpRecqYrSuOCTGy2YOYwdag6Jg==; 31:UHeWeus3oH6zYIU9ggbSA/7xGn77QlRqzSUoM4yhGsNFIO5PFvkA/8Wb195x7NVloCNgGpXa4nb/wnQJOapOFpk4212j8xN4WPMogtFcaE7BYbzwfRK9ufL/9OFXxhqezaOzGq6uZJCYomLt8dlzEd8PZGmT5tOwLij4U5QBdHIqfH94MpckSFu5zq/4yzkyOTXPlrxbmlhVUz3a5Zz0qZ7aaQ90fzMqBzPgCBt/qiw= X-MS-TrafficTypeDiagnostic: DBXPR07MB318: X-Microsoft-Exchange-Diagnostics: 1; DBXPR07MB318; 20:Xa43FZMfYtCStyd7KsRJv7bAxnRFia7pNzINkckVbWfurbC8/0Jdqc3XLigaBPiWX+xsIz4x8hrxyFMCj+qYQBrsC1SlyQoKNF5VK9MvDThiDR0zNEqhvqSTMklq5JQ0Y7SZAFDw1wFCh9tDKYgR/06ZNbE+OJqOjvL/s97fBBvLc9aKnptn9mROVC9DUkMwZncDoZ9cHSWVUSlKnSq8UMYGLLOJJg5YYmyhJ9Kh/yQF4LT1ltQfPBNHaC3j38aVXfB+t53TtDCEqrHfNzQMheqTCETa8WCpve1ipSlB2ufcp/yHZJM7M/IPSgfHoo5rSzNGC4xfoZ1V4B1yxK2DF/YL6PP6pUfEIa6Zy8RWgel+Kk9mR1i2KxHXbhXJMX1JjgW8c7OgGfvQiNUWGNezVE65Kc8yrFCnDU6eE4O41yeCpxa5mNdDfyBaNOMHCwSnybcPFO5p7nqtkfaYg1qArsfDQj4QOpNgGqxqItQ+xBhI5xpkU39sT4VlupqckVuj; 4:Yb4I+utbVGDBhwsD7I+QQQOOjLhTO8u116uHKzz2oeFhveCc7LmWuJJDON1RcFI8WkFa5UgZPd5YS05zJMr0BKAGr/sQcosgeZ2LUfsq1Y03d/Lm5kW98qK/q0P/rD+xlQ3+s0hasNAo+Wi5TZS1CQ0Cm9ReY+FVj9CYivrdd1AnrRMc9thqy9yiVonS1v6Sqno7zO0POiNXJ5COP+CwmxlQxZno0cPa4BpeNqhg5Ak66ZBecpmk8umQ0EgL6camEzjisyCzsmDj6EBHyDFd+Tj7EJMjiJsBE0HSmlyyri0= X-Exchange-Antispam-Report-Test: UriScan:(37575265505322); X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(601004)(2401047)(5005006)(8121501046)(100000703101)(100105400095)(3002001)(10201501046)(93006095)(93001095)(6041248)(20161123555025)(20161123560025)(20161123564025)(20161123562025)(20161123558100)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095); SRVR:DBXPR07MB318; BCL:0; PCL:0; RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095); SRVR:DBXPR07MB318; X-Forefront-PRVS: 04163EF38A X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(7370300001)(4630300001)(6009001)(6049001)(199003)(189002)(65956001)(66066001)(65806001)(68736007)(50466002)(229853002)(54356999)(305945005)(76176999)(50986999)(42186005)(7350300001)(4326008)(81156014)(97736004)(6116002)(86362001)(31686004)(36756003)(478600001)(81166006)(117156002)(4001350100001)(31696002)(64126003)(47776003)(189998001)(83506001)(5660300001)(53936002)(6666003)(8676002)(23676002)(230700001)(3846002)(65826007)(2906002)(6246003)(53546010)(2950100002)(7736002)(54906002)(33646002)(25786009)(77096006)(105586002)(6486002)(106356001)(101416001); DIR:OUT; SFP:1101; SCL:1; SRVR:DBXPR07MB318; H:[192.168.0.102]; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; Received-SPF: None (protection.outlook.com: ericsson.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtEQlhQUjA3TUIzMTg7MjM6dnhKUjlNZWhyNHpWWWtucU5NZGlhUkVHdUNm?= =?utf-8?B?M0o3cEFqL0pQU0pFQUVvNytzZFE5bFBQbmdSS0JYeVAvSHN1V3djUkVyajVx?= =?utf-8?B?ZjFlTkNOR0htU0F1RXhMbmdGMXJRYmR4SWlndklEUW5LOGZIakRySC9zREw4?= =?utf-8?B?UmlYYXFmQU9vZm1pTk9WTm9qSEV0ekNCT3NDZVBUMmJKeTBYY1ZjZUR0SDZk?= =?utf-8?B?bWVsOEI1aWhqNFhTL0p2V0tGblhYQjF4S0lNWUFUUDMvb0N0YWJkcmVidzFs?= =?utf-8?B?bU1MZllkcm9KM1JNMEg2WkRtd2tKSmlid3paVVF4eEVHUkZuVWY0a0RBbHNL?= =?utf-8?B?QkQxVlR1ZTBIWmQ5QzVlVHlBWVJrWUxuTGl0QXFIc0FvNnNwekdiUUpTUFIv?= =?utf-8?B?SEJXZGFPcWY4cXY2Z3hOM3lIUmdzWVFpa0R6SmJwZWRKUm16cEVxd0Q5cjdR?= =?utf-8?B?VjYzdEx4cm9SeFVKekZPWGJvc2poK01xdUFEVHlKK0xyeDZBbVZ0Z09RekdO?= =?utf-8?B?Y1h4aE5pWHB2QjN1SDN3MFJzeG9pckNtaXpNcWZrcEhjSUFlMU5xZ0cwUWJh?= =?utf-8?B?MWlhbU5lWHhnUFVNQnZEdGxDbm9UaWZZSjc5R2V6R1BFOVZiRjd6TXZYL2tV?= =?utf-8?B?cUl5K0dmbGc5NHNwYnAvSjNCRHE0dUhzQ1B6MjZGT01pREcwSHd6eGJwMDNE?= =?utf-8?B?Zm9MdXJtTGxvUG5Ec0cxNmdHSXgrNklkOWYvK1NMY0gwczgwSFdFR3c0NmZD?= =?utf-8?B?UlVlQVp6NlUzR3Vla3RkajcyQTZuWWVUWXNkekExOGVYalNxMCtNMUZrRW5N?= =?utf-8?B?ZEtueEE2cVd4QnRGajJTNGRVZmRoNzR6bE91S21MbzQ0K3RoUm9lNE9sbDNY?= =?utf-8?B?K3p6NDRmUE9UU1htNTI3RUFQajhTY1NnVWFzT24zejNrdGdLYmtoQ1phK1lw?= =?utf-8?B?ZlZVd0NKVnZNSEh6eU1iTGZSYXJybXlySnFYa3lDUUMxNzZYVENLcWYwMTQ5?= =?utf-8?B?bmlmMWRhak5EU3RlbGcvN0F4NUk1ZEhZYy9VeEd0TkI2U3p4VEtCenJmcEpr?= =?utf-8?B?OVNIWnp2WmYvQlp5bXFUN1NFWEpONWVRaWwrbzhhdDRNOTB3MzhSSEpXWDUx?= =?utf-8?B?RGl3V0w4UHVLWDA4MmNRZFc5T3hueklLOElZeklZS0hiY0NDWjFYLytmTCs1?= =?utf-8?B?eTdTd0hlaFNacDZ5MzdZd3lWOURLcXRDa1IvTDQzRHdmaXNyRS9QZndFZWRK?= =?utf-8?B?c2pMbHpmaG4zVVlTUXRYWkdvUnhObnBSK0lyM3U1R3dSSnRjelRzbmQ5QkdM?= =?utf-8?B?eEVEbmRqakxoMjVJSDRWY0MvTm9oSktlOCtJN256NGoxaVJmenZFNVBSSjk5?= =?utf-8?B?VHk3bWNhRTdLMzBZSUhaVXJRMHlGNnJpNmh3b3gvWHBEVzUyNkxxWmhxNWNo?= =?utf-8?B?MnpGc1NmcGtMM2grYy9kbWhvZ0Zud0xITGNUNk1JM3NyMDREazZmaW5EK0Nh?= =?utf-8?B?TmtwdjZPK1U1YVFOTGNKeWdHNGtXNzFoV1I1NDdGdXo4aGErbndlaW9uZkVT?= =?utf-8?B?bHlBV2laUnFtTzVad2M4cUhrYUJoZEdnYzZmdTdqYTd2YjFOeGx0dXd6MGRS?= =?utf-8?B?UFVJZkNtaStsbmVDSWcrcE1QcjY4N1JhWlFjemFwc1Buc1p1QTliV3Yza0FE?= =?utf-8?B?S1lYbnJaVGkrMjFNZWRqbWNkdDd5Z1NRb24rdml5cHF5UVFQWWJPUTg5M1V3?= =?utf-8?Q?c9Te3gEr8dD1Sfy9J5OWjJHxwsMFa//KNhh8=3D?= X-Microsoft-Exchange-Diagnostics: 1; DBXPR07MB318; 6:MyAPXegWZFKUmLx1CjUHrNDjcUGtWqw1Qg2mgqZ9G0fPwLrDK9+CAp8m1yjhXu3VmtIP2aradn+jNRyE8GwBVDkiF1Q2o8qSRFJymZ3KP5kJsMyL/qVyovYnB5qcUqt2SPsNDw06Bpw8tWPXhfdmJM/W1oZ3JTXtmujFlK8/m5XhlHL+svZ5CjsljgiJqtiof4h4s2YaqLDH96VXVTUkwenGsc17oLC3gyFtzZ/2Nbnf3qvFYkA9o6DDqIt2MmfxzF7rft0X8b8xbAOfDO1jHSKimx5+RzFnrovObcfh8hHPCvJUx8vCiwZIngKYWCgP8G68zbIFbR5bljC3/BGCSQ==; 5:/Km3Pd+jZi69NHWYSyUKZXsFjqTdBeAtf95teFpXAL8+iPukmXPk4vgFhVMvZdTtwuFK4z8GRKWXs0SfiNjJABVq7Cf7yWhktwFZ0jOz5q/5r/bioKO8+Q03695Lu5L9v9fWhTPahnBSdP6DIkj9qQ==; 24:Lv1qJd+zUKtHgsLRnNK1DCT35flyx7g1+SVpapThVuz1h0n+qylnZG/f6AE22YESpZKWC8dZjSxYX9+8XsFak6EwuVYhdW3GVNLPfVKvB6M=; 7:MVC3XXbsqRO6CDKlhst368mU/qMbiaKU2i+JHty+LW89NSR3DwM5ADLVoWFdhYiQkT1MK7idkx7P0jOq0NbY4xHY00Ks2qkAQ6fDu79oR5D2V9WUgIzOcuVxMUCIw1g9R/ImaSxBMsM1JiZxcvIIlxmo2Q2NawpJseL3Gtv6SGhUuP1A2mlFLoYQzcf6DKeteasxyrxYdCaIkqI3xqHCTTd78THKuX1KGtRM2j8el5Y= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Aug 2017 19:37:48.4359 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DBXPR07MB318 X-OriginatorOrg: ericsson.com X-IsSubscribed: yes > diff --git a/gdb/unittests/environ-selftests.c b/gdb/unittests/environ-selftests.c > index 1e938e6746..e2d8c70985 100644 > --- a/gdb/unittests/environ-selftests.c > +++ b/gdb/unittests/environ-selftests.c > @@ -22,132 +22,257 @@ > #include "common/environ.h" > #include "common/diagnostics.h" > > +static bool > +set_contains (const std::set &set, std::string key) > +{ > + return set.find (key) != set.end (); > +} > + > namespace selftests { > namespace gdb_environ_tests { > > +/* Test if the vector is initialized in a correct way. */ > + > static void > -run_tests () > +test_vector_initialization (gdb_environ *env) > { > - /* Set a test environment variable. This will be unset at the end > - of this function. */ > - if (setenv ("GDB_SELFTEST_ENVIRON", "1", 1) != 0) > - error (_("Could not set environment variable for testing.")); > - > - gdb_environ env; > - > /* When the vector is initialized, there should always be one NULL > element in it. */ > - SELF_CHECK (env.envp ()[0] == NULL); > + SELF_CHECK (env->envp ()[0] == NULL); > + SELF_CHECK (env->user_set_env ().size () == 0); > + SELF_CHECK (env->user_unset_env ().size () == 0); > > /* Make sure that there is no other element. */ > - SELF_CHECK (env.get ("PWD") == NULL); > + SELF_CHECK (env->get ("PWD") == NULL); > +} > > - /* Check if unset followed by a set in an empty vector works. */ > - env.set ("PWD", "test"); > - SELF_CHECK (strcmp (env.get ("PWD"), "test") == 0); > - /* The second element must be NULL. */ > - SELF_CHECK (env.envp ()[1] == NULL); > - env.unset ("PWD"); > - SELF_CHECK (env.envp ()[0] == NULL); > +/* Test initialization using the host's environ. */ > > +static void > +test_init_from_host_environ (gdb_environ *env) > +{ > /* Initialize the environment vector using the host's environ. */ > - env = gdb_environ::from_host_environ (); > + *env = gdb_environ::from_host_environ (); > + > + /* The user-set and user-unset lists must be empty. */ > + SELF_CHECK (env->user_set_env ().size () == 0); > + SELF_CHECK (env->user_unset_env ().size () == 0); > > /* Our test environment variable should be present at the > vector. */ > - SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON"), "1") == 0); > - > - /* Set our test variable to another value. */ > - env.set ("GDB_SELFTEST_ENVIRON", "test"); > - SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON"), "test") == 0); > - > - /* And unset our test variable. The variable still exists in the > - host's environment, but doesn't exist in our vector. */ > - env.unset ("GDB_SELFTEST_ENVIRON"); > - SELF_CHECK (env.get ("GDB_SELFTEST_ENVIRON") == NULL); > - > - /* Re-set the test variable. */ > - env.set ("GDB_SELFTEST_ENVIRON", "1"); > - SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON"), "1") == 0); > + SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON"), "1") == 0); > +} > > - /* When we clear our environ vector, there should be only one > - element on it (NULL), and we shouldn't be able to get our test > - variable. */ > - env.clear (); > - SELF_CHECK (env.envp ()[0] == NULL); > - SELF_CHECK (env.get ("GDB_SELFTEST_ENVIRON") == NULL); > +/* Test reinitialization using the host's environ. */ > > +static void > +test_reinit_from_host_environ (gdb_environ *env) > +{ > /* Reinitialize our environ vector using the host environ. We > should be able to see one (and only one) instance of the test > variable. */ > - env = gdb_environ::from_host_environ (); > - char **envp = env.envp (); > + *env = gdb_environ::from_host_environ (); > + char **envp = env->envp (); > int num_found = 0; > > for (size_t i = 0; envp[i] != NULL; ++i) > if (strcmp (envp[i], "GDB_SELFTEST_ENVIRON=1") == 0) > ++num_found; > SELF_CHECK (num_found == 1); > +} > > - /* Get rid of our test variable. */ > - unsetenv ("GDB_SELFTEST_ENVIRON"); > +/* Test the case when we set a variable A, then set a variable B, > + then unset A, and make sure that we cannot find A in the environ > + vector, but can still find B. */ > + > +static void > +test_set_A_unset_B_unset_A_cannot_find_A_can_find_B (gdb_environ *env) > +{ > + env->set ("GDB_SELFTEST_ENVIRON_1", "aaa"); > + SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON_1"), "aaa") == 0); > + /* User-set environ var list must contain one element. */ > + SELF_CHECK (env->user_set_env ().size () == 1); > + SELF_CHECK (set_contains (env->user_set_env (), > + std::string ("GDB_SELFTEST_ENVIRON_1=aaa"))); > > - /* Test the case when we set a variable A, then set a variable B, > - then unset A, and make sure that we cannot find A in the environ > - vector, but can still find B. */ > - env.set ("GDB_SELFTEST_ENVIRON_1", "aaa"); > - SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON_1"), "aaa") == 0); > + env->set ("GDB_SELFTEST_ENVIRON_2", "bbb"); > + SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON_2"), "bbb") == 0); > > - env.set ("GDB_SELFTEST_ENVIRON_2", "bbb"); > - SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON_2"), "bbb") == 0); > + env->unset ("GDB_SELFTEST_ENVIRON_1"); > + SELF_CHECK (env->get ("GDB_SELFTEST_ENVIRON_1") == NULL); > + SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON_2"), "bbb") == 0); > > - env.unset ("GDB_SELFTEST_ENVIRON_1"); > - SELF_CHECK (env.get ("GDB_SELFTEST_ENVIRON_1") == NULL); > - SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON_2"), "bbb") == 0); > + /* The user-set environ var list must contain only one element > + now. */ > + SELF_CHECK (set_contains (env->user_set_env (), > + std::string ("GDB_SELFTEST_ENVIRON_2=bbb"))); > + SELF_CHECK (env->user_set_env ().size () == 1); > +} > > - env.clear (); > +/* Check if unset followed by a set in an empty vector works. */ > > - /* Test that after a std::move the moved-from object is left at a > - valid state (i.e., its only element is NULL). */ > - env.set ("A", "1"); > - SELF_CHECK (strcmp (env.get ("A"), "1") == 0); > +static void > +test_unset_set_empty_vector (gdb_environ *env) > +{ > + env->set ("PWD", "test"); > + SELF_CHECK (strcmp (env->get ("PWD"), "test") == 0); > + SELF_CHECK (set_contains (env->user_set_env (), std::string ("PWD=test"))); > + SELF_CHECK (env->user_unset_env ().size () == 0); > + /* The second element must be NULL. */ > + SELF_CHECK (env->envp ()[1] == NULL); > + SELF_CHECK (env->user_set_env ().size () == 1); > + env->unset ("PWD"); > + SELF_CHECK (env->envp ()[0] == NULL); > + SELF_CHECK (env->user_set_env ().size () == 0); > + SELF_CHECK (env->user_unset_env ().size () == 1); > + SELF_CHECK (set_contains (env->user_unset_env (), std::string ("PWD"))); > +} > + > +/* When we clear our environ vector, there should be only one > + element on it (NULL), and we shouldn't be able to get our test > + variable. */ > + > +static void > +test_vector_clear (gdb_environ *env) > +{ > + env->clear (); > + SELF_CHECK (env->envp ()[0] == NULL); > + SELF_CHECK (env->user_set_env ().size () == 0); > + SELF_CHECK (env->user_unset_env ().size () == 0); > + SELF_CHECK (env->get ("GDB_SELFTEST_ENVIRON") == NULL); > +} > + > +/* Test that after a std::move the moved-from object is left at a > + valid state (i.e., its only element is NULL). */ > + > +static void > +test_std_move (gdb_environ *env) > +{ > + env->set ("A", "1"); > + SELF_CHECK (strcmp (env->get ("A"), "1") == 0); > + SELF_CHECK (set_contains (env->user_set_env (), std::string ("A=1"))); > + SELF_CHECK (env->user_set_env ().size () == 1); > gdb_environ env2; > - env2 = std::move (env); > - SELF_CHECK (env.envp ()[0] == NULL); > + env2 = std::move (*env); > + SELF_CHECK (env->envp ()[0] == NULL); > SELF_CHECK (strcmp (env2.get ("A"), "1") == 0); > SELF_CHECK (env2.envp ()[1] == NULL); > - env.set ("B", "2"); > - SELF_CHECK (strcmp (env.get ("B"), "2") == 0); > - SELF_CHECK (env.envp ()[1] == NULL); > + SELF_CHECK (env->user_set_env ().size () == 0); > + SELF_CHECK (set_contains (env2.user_set_env (), std::string ("A=1"))); > + SELF_CHECK (env2.user_set_env ().size () == 1); > + env->set ("B", "2"); > + SELF_CHECK (strcmp (env->get ("B"), "2") == 0); > + SELF_CHECK (env->envp ()[1] == NULL); > +} > > - /* Test that the move constructor leaves everything at a valid > - state. */ > - env.clear (); > - env.set ("A", "1"); > - SELF_CHECK (strcmp (env.get ("A"), "1") == 0); > - gdb_environ env3 = std::move (env); > - SELF_CHECK (env.envp ()[0] == NULL); > +/* Test that the move constructor leaves everything at a valid > + state. */ > + > +static void > +test_move_constructor (gdb_environ *env) > +{ > + env->clear (); > + env->set ("A", "1"); > + SELF_CHECK (strcmp (env->get ("A"), "1") == 0); > + SELF_CHECK (set_contains (env->user_set_env (), std::string ("A=1"))); > + gdb_environ env3 = std::move (*env); > + SELF_CHECK (env->envp ()[0] == NULL); > + SELF_CHECK (env->user_set_env ().size () == 0); > SELF_CHECK (strcmp (env3.get ("A"), "1") == 0); > SELF_CHECK (env3.envp ()[1] == NULL); > - env.set ("B", "2"); > - SELF_CHECK (strcmp (env.get ("B"), "2") == 0); > - SELF_CHECK (env.envp ()[1] == NULL); > + SELF_CHECK (set_contains (env3.user_set_env (), std::string ("A=1"))); > + SELF_CHECK (env3.user_set_env ().size () == 1); > + env->set ("B", "2"); > + SELF_CHECK (strcmp (env->get ("B"), "2") == 0); > + SELF_CHECK (env->envp ()[1] == NULL); > + SELF_CHECK (set_contains (env->user_set_env (), std::string ("B=2"))); > + SELF_CHECK (env->user_set_env ().size () == 1); > +} > + > +/* Test that self-moving works. */ > > +static void > +test_self_move (gdb_environ *env) > +{ > /* Test self-move. */ > - env.clear (); > - env.set ("A", "1"); > - SELF_CHECK (strcmp (env.get ("A"), "1") == 0); > + env->clear (); > + env->set ("A", "1"); > + SELF_CHECK (strcmp (env->get ("A"), "1") == 0); > + SELF_CHECK (set_contains (env->user_set_env (), std::string ("A=1"))); > + SELF_CHECK (env->user_set_env ().size () == 1); > > /* Some compilers warn about moving to self, but that's precisely what we want > to test here, so turn this warning off. */ > DIAGNOSTIC_PUSH > DIAGNOSTIC_IGNORE_SELF_MOVE > - env = std::move (env); > + *env = std::move (*env); > DIAGNOSTIC_POP > > - SELF_CHECK (strcmp (env.get ("A"), "1") == 0); > - SELF_CHECK (strcmp (env.envp ()[0], "A=1") == 0); > - SELF_CHECK (env.envp ()[1] == NULL); > + SELF_CHECK (strcmp (env->get ("A"), "1") == 0); > + SELF_CHECK (strcmp (env->envp ()[0], "A=1") == 0); > + SELF_CHECK (env->envp ()[1] == NULL); > + SELF_CHECK (set_contains (env->user_set_env (), std::string ("A=1"))); > + SELF_CHECK (env->user_set_env ().size () == 1); > +} > + > +/* Test if set/unset/reset works. */ > + > +static void > +test_set_unset_reset (gdb_environ *env) > +{ > + /* Set our test variable to another value. */ > + env->set ("GDB_SELFTEST_ENVIRON", "test"); > + SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON"), "test") == 0); > + SELF_CHECK (env->user_set_env ().size () == 1); > + SELF_CHECK (env->user_unset_env ().size () == 0); > + > + /* And unset our test variable. The variable still exists in the > + host's environment, but doesn't exist in our vector. */ > + env->unset ("GDB_SELFTEST_ENVIRON"); > + SELF_CHECK (env->get ("GDB_SELFTEST_ENVIRON") == NULL); > + SELF_CHECK (env->user_unset_env ().size () == 1); > + SELF_CHECK (set_contains (env->user_unset_env (), > + std::string ("GDB_SELFTEST_ENVIRON"))); > + > + /* Re-set the test variable. */ > + env->set ("GDB_SELFTEST_ENVIRON", "1"); > + SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON"), "1") == 0); > +} > + > +static void > +run_tests () > +{ > + /* Set a test environment variable. This will be unset at the end > + of this function. */ > + if (setenv ("GDB_SELFTEST_ENVIRON", "1", 1) != 0) > + error (_("Could not set environment variable for testing.")); > + > + gdb_environ env; > + > + test_vector_initialization (&env); > + > + test_unset_set_empty_vector (&env); > + > + test_init_from_host_environ (&env); > + > + test_set_unset_reset (&env); > + > + test_vector_clear (&env); > + > + test_reinit_from_host_environ (&env); > + > + /* Get rid of our test variable. We won't need it anymore. */ > + unsetenv ("GDB_SELFTEST_ENVIRON"); > + > + test_set_A_unset_B_unset_A_cannot_find_A_can_find_B (&env); > + > + env.clear (); > + > + test_std_move (&env); > + > + test_move_constructor (&env); > + > + test_self_move (&env); > } > } /* namespace gdb_environ */ > } /* namespace selftests */ > Hi Sergio, The main point of splitting that long test into separate functions is to have each of them independent from each other. So they should ideally not use the same environment object. I suggest doing something like this (applied on top of your patch). I did this super quickly, so please check that the tests still make sense :) From 0a7d0381844d640792cc3f3051f643db8df42937 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Tue, 29 Aug 2017 17:06:51 +0200 Subject: [PATCH] stuff --- gdb/unittests/environ-selftests.c | 238 +++++++++++++++++++++----------------- 1 file changed, 130 insertions(+), 108 deletions(-) diff --git a/gdb/unittests/environ-selftests.c b/gdb/unittests/environ-selftests.c index e2d8c70..72a46af 100644 --- a/gdb/unittests/environ-selftests.c +++ b/gdb/unittests/environ-selftests.c @@ -22,6 +22,8 @@ #include "common/environ.h" #include "common/diagnostics.h" +static const char *gdb_selftest_env_var = "GDB_SELFTEST_ENVIRON"; + static bool set_contains (const std::set &set, std::string key) { @@ -34,45 +36,48 @@ namespace gdb_environ_tests { /* Test if the vector is initialized in a correct way. */ static void -test_vector_initialization (gdb_environ *env) +test_vector_initialization () { + gdb_environ env; + /* When the vector is initialized, there should always be one NULL element in it. */ - SELF_CHECK (env->envp ()[0] == NULL); - SELF_CHECK (env->user_set_env ().size () == 0); - SELF_CHECK (env->user_unset_env ().size () == 0); + SELF_CHECK (env.envp ()[0] == NULL); + SELF_CHECK (env.user_set_env ().size () == 0); + SELF_CHECK (env.user_unset_env ().size () == 0); /* Make sure that there is no other element. */ - SELF_CHECK (env->get ("PWD") == NULL); + SELF_CHECK (env.get ("PWD") == NULL); } /* Test initialization using the host's environ. */ static void -test_init_from_host_environ (gdb_environ *env) +test_init_from_host_environ () { /* Initialize the environment vector using the host's environ. */ - *env = gdb_environ::from_host_environ (); + gdb_environ env = gdb_environ::from_host_environ (); /* The user-set and user-unset lists must be empty. */ - SELF_CHECK (env->user_set_env ().size () == 0); - SELF_CHECK (env->user_unset_env ().size () == 0); + SELF_CHECK (env.user_set_env ().size () == 0); + SELF_CHECK (env.user_unset_env ().size () == 0); /* Our test environment variable should be present at the vector. */ - SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON"), "1") == 0); + SELF_CHECK (strcmp (env.get (gdb_selftest_env_var), "1") == 0); } /* Test reinitialization using the host's environ. */ static void -test_reinit_from_host_environ (gdb_environ *env) +test_reinit_from_host_environ () { /* Reinitialize our environ vector using the host environ. We should be able to see one (and only one) instance of the test variable. */ - *env = gdb_environ::from_host_environ (); - char **envp = env->envp (); + gdb_environ env = gdb_environ::from_host_environ (); + env = gdb_environ::from_host_environ (); + char **envp = env.envp (); int num_found = 0; for (size_t i = 0; envp[i] != NULL; ++i) @@ -86,46 +91,50 @@ test_reinit_from_host_environ (gdb_environ *env) vector, but can still find B. */ static void -test_set_A_unset_B_unset_A_cannot_find_A_can_find_B (gdb_environ *env) +test_set_A_unset_B_unset_A_cannot_find_A_can_find_B () { - env->set ("GDB_SELFTEST_ENVIRON_1", "aaa"); - SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON_1"), "aaa") == 0); + gdb_environ env; + + env.set ("GDB_SELFTEST_ENVIRON_1", "aaa"); + SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON_1"), "aaa") == 0); /* User-set environ var list must contain one element. */ - SELF_CHECK (env->user_set_env ().size () == 1); - SELF_CHECK (set_contains (env->user_set_env (), + SELF_CHECK (env.user_set_env ().size () == 1); + SELF_CHECK (set_contains (env.user_set_env (), std::string ("GDB_SELFTEST_ENVIRON_1=aaa"))); - env->set ("GDB_SELFTEST_ENVIRON_2", "bbb"); - SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON_2"), "bbb") == 0); + env.set ("GDB_SELFTEST_ENVIRON_2", "bbb"); + SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON_2"), "bbb") == 0); - env->unset ("GDB_SELFTEST_ENVIRON_1"); - SELF_CHECK (env->get ("GDB_SELFTEST_ENVIRON_1") == NULL); - SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON_2"), "bbb") == 0); + env.unset ("GDB_SELFTEST_ENVIRON_1"); + SELF_CHECK (env.get ("GDB_SELFTEST_ENVIRON_1") == NULL); + SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON_2"), "bbb") == 0); /* The user-set environ var list must contain only one element now. */ - SELF_CHECK (set_contains (env->user_set_env (), + SELF_CHECK (set_contains (env.user_set_env (), std::string ("GDB_SELFTEST_ENVIRON_2=bbb"))); - SELF_CHECK (env->user_set_env ().size () == 1); + SELF_CHECK (env.user_set_env ().size () == 1); } /* Check if unset followed by a set in an empty vector works. */ static void -test_unset_set_empty_vector (gdb_environ *env) +test_unset_set_empty_vector () { - env->set ("PWD", "test"); - SELF_CHECK (strcmp (env->get ("PWD"), "test") == 0); - SELF_CHECK (set_contains (env->user_set_env (), std::string ("PWD=test"))); - SELF_CHECK (env->user_unset_env ().size () == 0); + gdb_environ env; + + env.set ("PWD", "test"); + SELF_CHECK (strcmp (env.get ("PWD"), "test") == 0); + SELF_CHECK (set_contains (env.user_set_env (), std::string ("PWD=test"))); + SELF_CHECK (env.user_unset_env ().size () == 0); /* The second element must be NULL. */ - SELF_CHECK (env->envp ()[1] == NULL); - SELF_CHECK (env->user_set_env ().size () == 1); - env->unset ("PWD"); - SELF_CHECK (env->envp ()[0] == NULL); - SELF_CHECK (env->user_set_env ().size () == 0); - SELF_CHECK (env->user_unset_env ().size () == 1); - SELF_CHECK (set_contains (env->user_unset_env (), std::string ("PWD"))); + SELF_CHECK (env.envp ()[1] == NULL); + SELF_CHECK (env.user_set_env ().size () == 1); + env.unset ("PWD"); + SELF_CHECK (env.envp ()[0] == NULL); + SELF_CHECK (env.user_set_env ().size () == 0); + SELF_CHECK (env.user_unset_env ().size () == 1); + SELF_CHECK (set_contains (env.user_unset_env (), std::string ("PWD"))); } /* When we clear our environ vector, there should be only one @@ -133,110 +142,127 @@ test_unset_set_empty_vector (gdb_environ *env) variable. */ static void -test_vector_clear (gdb_environ *env) +test_vector_clear () { - env->clear (); - SELF_CHECK (env->envp ()[0] == NULL); - SELF_CHECK (env->user_set_env ().size () == 0); - SELF_CHECK (env->user_unset_env ().size () == 0); - SELF_CHECK (env->get ("GDB_SELFTEST_ENVIRON") == NULL); + gdb_environ env; + + env.set (gdb_selftest_env_var, "1"); + + env.clear (); + SELF_CHECK (env.envp ()[0] == NULL); + SELF_CHECK (env.user_set_env ().size () == 0); + SELF_CHECK (env.user_unset_env ().size () == 0); + SELF_CHECK (env.get (gdb_selftest_env_var) == NULL); } /* Test that after a std::move the moved-from object is left at a valid state (i.e., its only element is NULL). */ static void -test_std_move (gdb_environ *env) +test_std_move () { - env->set ("A", "1"); - SELF_CHECK (strcmp (env->get ("A"), "1") == 0); - SELF_CHECK (set_contains (env->user_set_env (), std::string ("A=1"))); - SELF_CHECK (env->user_set_env ().size () == 1); + gdb_environ env; gdb_environ env2; - env2 = std::move (*env); - SELF_CHECK (env->envp ()[0] == NULL); + + env.set ("A", "1"); + SELF_CHECK (strcmp (env.get ("A"), "1") == 0); + SELF_CHECK (set_contains (env.user_set_env (), std::string ("A=1"))); + SELF_CHECK (env.user_set_env ().size () == 1); + + env2 = std::move (env); + SELF_CHECK (env.envp ()[0] == NULL); SELF_CHECK (strcmp (env2.get ("A"), "1") == 0); SELF_CHECK (env2.envp ()[1] == NULL); - SELF_CHECK (env->user_set_env ().size () == 0); + SELF_CHECK (env.user_set_env ().size () == 0); SELF_CHECK (set_contains (env2.user_set_env (), std::string ("A=1"))); SELF_CHECK (env2.user_set_env ().size () == 1); - env->set ("B", "2"); - SELF_CHECK (strcmp (env->get ("B"), "2") == 0); - SELF_CHECK (env->envp ()[1] == NULL); + env.set ("B", "2"); + SELF_CHECK (strcmp (env.get ("B"), "2") == 0); + SELF_CHECK (env.envp ()[1] == NULL); } /* Test that the move constructor leaves everything at a valid state. */ static void -test_move_constructor (gdb_environ *env) +test_move_constructor () { - env->clear (); - env->set ("A", "1"); - SELF_CHECK (strcmp (env->get ("A"), "1") == 0); - SELF_CHECK (set_contains (env->user_set_env (), std::string ("A=1"))); - gdb_environ env3 = std::move (*env); - SELF_CHECK (env->envp ()[0] == NULL); - SELF_CHECK (env->user_set_env ().size () == 0); - SELF_CHECK (strcmp (env3.get ("A"), "1") == 0); - SELF_CHECK (env3.envp ()[1] == NULL); - SELF_CHECK (set_contains (env3.user_set_env (), std::string ("A=1"))); - SELF_CHECK (env3.user_set_env ().size () == 1); - env->set ("B", "2"); - SELF_CHECK (strcmp (env->get ("B"), "2") == 0); - SELF_CHECK (env->envp ()[1] == NULL); - SELF_CHECK (set_contains (env->user_set_env (), std::string ("B=2"))); - SELF_CHECK (env->user_set_env ().size () == 1); + gdb_environ env; + + env.set ("A", "1"); + SELF_CHECK (strcmp (env.get ("A"), "1") == 0); + SELF_CHECK (set_contains (env.user_set_env (), std::string ("A=1"))); + + gdb_environ env2 = std::move (env); + SELF_CHECK (env.envp ()[0] == NULL); + SELF_CHECK (env.user_set_env ().size () == 0); + SELF_CHECK (strcmp (env2.get ("A"), "1") == 0); + SELF_CHECK (env2.envp ()[1] == NULL); + SELF_CHECK (set_contains (env2.user_set_env (), std::string ("A=1"))); + SELF_CHECK (env2.user_set_env ().size () == 1); + + env.set ("B", "2"); + SELF_CHECK (strcmp (env.get ("B"), "2") == 0); + SELF_CHECK (env.envp ()[1] == NULL); + SELF_CHECK (set_contains (env.user_set_env (), std::string ("B=2"))); + SELF_CHECK (env.user_set_env ().size () == 1); } /* Test that self-moving works. */ static void -test_self_move (gdb_environ *env) +test_self_move () { + gdb_environ env; + /* Test self-move. */ - env->clear (); - env->set ("A", "1"); - SELF_CHECK (strcmp (env->get ("A"), "1") == 0); - SELF_CHECK (set_contains (env->user_set_env (), std::string ("A=1"))); - SELF_CHECK (env->user_set_env ().size () == 1); + env.set ("A", "1"); + SELF_CHECK (strcmp (env.get ("A"), "1") == 0); + SELF_CHECK (set_contains (env.user_set_env (), std::string ("A=1"))); + SELF_CHECK (env.user_set_env ().size () == 1); /* Some compilers warn about moving to self, but that's precisely what we want to test here, so turn this warning off. */ DIAGNOSTIC_PUSH DIAGNOSTIC_IGNORE_SELF_MOVE - *env = std::move (*env); + env = std::move (env); DIAGNOSTIC_POP - SELF_CHECK (strcmp (env->get ("A"), "1") == 0); - SELF_CHECK (strcmp (env->envp ()[0], "A=1") == 0); - SELF_CHECK (env->envp ()[1] == NULL); - SELF_CHECK (set_contains (env->user_set_env (), std::string ("A=1"))); - SELF_CHECK (env->user_set_env ().size () == 1); + SELF_CHECK (strcmp (env.get ("A"), "1") == 0); + SELF_CHECK (strcmp (env.envp ()[0], "A=1") == 0); + SELF_CHECK (env.envp ()[1] == NULL); + SELF_CHECK (set_contains (env.user_set_env (), std::string ("A=1"))); + SELF_CHECK (env.user_set_env ().size () == 1); } /* Test if set/unset/reset works. */ static void -test_set_unset_reset (gdb_environ *env) +test_set_unset_reset () { + gdb_environ env = gdb_environ::from_host_environ (); + + /* Our test variable should already be present in the host's environment. */ + SELF_CHECK (env.get ("GDB_SELFTEST_ENVIRON") != NULL); + /* Set our test variable to another value. */ - env->set ("GDB_SELFTEST_ENVIRON", "test"); - SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON"), "test") == 0); - SELF_CHECK (env->user_set_env ().size () == 1); - SELF_CHECK (env->user_unset_env ().size () == 0); + env.set ("GDB_SELFTEST_ENVIRON", "test"); + SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON"), "test") == 0); + SELF_CHECK (env.user_set_env ().size () == 1); + SELF_CHECK (env.user_unset_env ().size () == 0); /* And unset our test variable. The variable still exists in the host's environment, but doesn't exist in our vector. */ - env->unset ("GDB_SELFTEST_ENVIRON"); - SELF_CHECK (env->get ("GDB_SELFTEST_ENVIRON") == NULL); - SELF_CHECK (env->user_unset_env ().size () == 1); - SELF_CHECK (set_contains (env->user_unset_env (), + env.unset ("GDB_SELFTEST_ENVIRON"); + SELF_CHECK (env.get ("GDB_SELFTEST_ENVIRON") == NULL); + SELF_CHECK (env.user_set_env ().size () == 0); + SELF_CHECK (env.user_unset_env ().size () == 1); + SELF_CHECK (set_contains (env.user_unset_env (), std::string ("GDB_SELFTEST_ENVIRON"))); /* Re-set the test variable. */ - env->set ("GDB_SELFTEST_ENVIRON", "1"); - SELF_CHECK (strcmp (env->get ("GDB_SELFTEST_ENVIRON"), "1") == 0); + env.set ("GDB_SELFTEST_ENVIRON", "1"); + SELF_CHECK (strcmp (env.get ("GDB_SELFTEST_ENVIRON"), "1") == 0); } static void @@ -247,32 +273,28 @@ run_tests () if (setenv ("GDB_SELFTEST_ENVIRON", "1", 1) != 0) error (_("Could not set environment variable for testing.")); - gdb_environ env; + test_vector_initialization (); - test_vector_initialization (&env); + test_unset_set_empty_vector (); - test_unset_set_empty_vector (&env); + test_init_from_host_environ (); - test_init_from_host_environ (&env); + test_set_unset_reset (); - test_set_unset_reset (&env); + test_vector_clear (); - test_vector_clear (&env); - - test_reinit_from_host_environ (&env); + test_reinit_from_host_environ (); /* Get rid of our test variable. We won't need it anymore. */ unsetenv ("GDB_SELFTEST_ENVIRON"); - test_set_A_unset_B_unset_A_cannot_find_A_can_find_B (&env); - - env.clear (); + test_set_A_unset_B_unset_A_cannot_find_A_can_find_B (); - test_std_move (&env); + test_std_move (); - test_move_constructor (&env); + test_move_constructor (); - test_self_move (&env); + test_self_move (); } } /* namespace gdb_environ */ } /* namespace selftests */