This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v5] gdb: ADI support
- From: Yao Qi <qiyaoltc at gmail dot com>
- To: Weimin Pan <weimin dot pan at oracle dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Thu, 27 Jul 2017 16:49:48 +0100
- Subject: Re: [PATCH v5] gdb: ADI support
- Authentication-results: sourceware.org; auth=none
- References: <1501115399-33066-1-git-send-email-weimin.pan@oracle.com>
Weimin Pan <weimin.pan@oracle.com> 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<sparc64_adi_info *> a_proc_list;
a_proc_list is not a good variable name. How about adi_proc_list?
Can you use "std::forward_list<sparc64_adi_info>" 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<regcache *>", and I am
trying to fix it.
> +
> +/* 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. */
--
Yao (齐尧)