From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 88735 invoked by alias); 27 Jul 2017 16:44:21 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 88719 invoked by uid 89); 27 Jul 2017 16:44:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=Practices, capitalize, Standards, Coding X-HELO: userp1040.oracle.com Received: from userp1040.oracle.com (HELO userp1040.oracle.com) (156.151.31.81) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 27 Jul 2017 16:44:19 +0000 Received: from aserv0022.oracle.com (aserv0022.oracle.com [141.146.126.234]) by userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id v6RGiGVl003690 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 27 Jul 2017 16:44:17 GMT Received: from userv0121.oracle.com (userv0121.oracle.com [156.151.31.72]) by aserv0022.oracle.com (8.14.4/8.14.4) with ESMTP id v6RGiGd9029936 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 27 Jul 2017 16:44:16 GMT Received: from abhmp0017.oracle.com (abhmp0017.oracle.com [141.146.116.23]) by userv0121.oracle.com (8.14.4/8.13.8) with ESMTP id v6RGiF6k012597; Thu, 27 Jul 2017 16:44:15 GMT Received: from [10.159.130.167] (/10.159.130.167) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Thu, 27 Jul 2017 09:44:15 -0700 Subject: Re: [PATCH v5] gdb: ADI support To: Yao Qi Cc: gdb-patches@sourceware.org References: <1501115399-33066-1-git-send-email-weimin.pan@oracle.com> <868tj9riar.fsf@gmail.com> From: Wei-min Pan Message-ID: Date: Thu, 27 Jul 2017 16:44:00 -0000 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <868tj9riar.fsf@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2017-07/txt/msg00426.txt.bz2 On 7/27/2017 8:49 AM, Yao Qi wrote: > Weimin Pan writes: > >> + >> +/* ADI stat settings. */ >> +typedef struct >> +{ >> + /* The ADI block size. */ >> + unsigned long blksize; >> + >> + /* Number of bits used for an ADI version tag which can be >> + * used together with the shift value for an ADI version tag >> + * to encode or extract the ADI version value in a pointer. */ >> + unsigned long nbits; >> + >> + /* The maximum ADI version tag value supported. */ >> + int max_version; >> + >> + /* ADI version tag file. */ >> + int tag_fd; >> + >> + /* ADI availability check has been done. */ >> + bool checked_avail; >> + >> + /* ADI is available. */ >> + bool is_avail; >> + >> +} adi_stat_t; >> + >> +/* Per-process ADI stat info. */ >> + >> +typedef struct >> +{ >> + /* The process identifier. */ >> + pid_t pid; >> + >> + /* The ADI stat. */ >> + adi_stat_t stat; >> +} sparc64_adi_info; >> + >> +static std::forward_list a_proc_list; > a_proc_list is not a good variable name. How about adi_proc_list? It was originally named "adi_proc_list". I shortened it so I don't run into those "long source lines" :) Will change it back. > Can you use "std::forward_list" instead? I am reading > book "C++ Coding Standards: 101 Rules, Guidelines, and Best Practices", > and know rule 79 is "Store only values and smart pointers in > containers". I know we have "std::forward_list", and I am > trying to fix it. Will take a look. >> + >> +/* Find ADI info for process PID. */ >> + >> +static sparc64_adi_info * >> +find_adi_info (pid_t pid) >> +{ >> + sparc64_adi_info *proc; >> + >> + for ( auto it = a_proc_list.begin(); it != a_proc_list.end(); ++it >> ) > for (const auto &info : a_proc_list) > >> + if ((*it)->pid == pid) >> + return (*it); >> + >> + return NULL; >> +} >> + >> +/* Add ADI info for process PID. Returns newly allocated info >> + object. */ >> + >> +static sparc64_adi_info * >> +add_adi_info (pid_t pid) >> +{ >> + sparc64_adi_info *proc = XCNEW (sparc64_adi_info); >> + >> + proc->pid = pid; >> + proc->stat.is_avail = false; >> + proc->stat.checked_avail = false; >> + proc->stat.tag_fd = 0; >> + a_proc_list.push_front (proc); >> + >> + return proc; >> +} >> + >> +/* Get ADI info for process PID, creating one if it doesn't exist. */ >> + >> +static sparc64_adi_info * >> +get_adi_info_proc (pid_t pid) >> +{ >> + sparc64_adi_info *proc; >> + >> + proc = find_adi_info (pid); >> + if (proc == NULL) >> + proc = add_adi_info (pid); >> + >> + return proc; >> +} >> + >> +static adi_stat_t >> +get_adi_info (pid_t pid) >> +{ >> + sparc64_adi_info *proc; >> + >> + proc = get_adi_info_proc (pid); >> + return proc->stat; >> +} >> + >> +/* Is called when GDB is no longer debugging process PID. It >> + deletes data structure that keeps track of the ADI stat. */ >> + >> +void >> +sparc64_forget_process (pid_t pid) >> +{ >> + sparc64_adi_info *proc; >> + int target_errno; >> + >> + proc = find_adi_info (pid); >> + if (proc != NULL) >> + { >> + if (proc->stat.tag_fd > 0) >> + target_fileio_close (proc->stat.tag_fd, &target_errno); >> + a_proc_list.remove (proc); >> + xfree (proc); >> + } >> + >> +} >> + >> + >> +int main () >> +{ >> + char *haddr; >> + caddr_t vaddr; >> + int version; >> + >> + // test ISM > We have mixed comment styles, // and "/**/". Since it is a c code, we > can use /**/? > >> + int shmid = shmget (IPC_PRIVATE, SHMSIZE, IPC_CREAT | 0666); >> + if (shmid == -1) >> + exit(1); >> + char *shmaddr = (char *)shmat (shmid, NULL, 0x666 | SHM_RND); >> + if (shmaddr == (char *)-1) >> + { >> + shmctl (shmid, IPC_RMID, NULL); >> + exit(1); >> + } >> + // enable ADI on ISM segment > It is a sentence, so Capitalize 'E', and put "." at the end. > > /* Enable ADI on ISM segment. */ > OK, will update the test with your comments. Thanks.