On Sun, Dec 30, 2018 at 11:22 AM Jim Wilson <j...@sifive.com> wrote:
>
> On Sat, Dec 29, 2018 at 2:23 PM Richard Henderson
> <richard.hender...@linaro.org> wrote:
> > On 12/29/18 9:09 AM, Jim Wilson wrote:
> > > +int csr_register_map[] = {
> >
> > static const?
>
> If I add static const here, then I get a build error if this patch is
> applied to the tree but the following patch #5 that uses the variable
> is not applied. Though I suppose I could fix that if I put the static
> const in patch 5. That would look a little funny but would work.
>
> > Putting an initialized variable in a header file doesn't seem right. Is
> > this
> > supposed to be a declaration that is shared between c files?
>
> I did it this way for two reasons. It makes it easier to keep the
> register mapping consistent with the gdb csr file, if the gdb csr file
> happens to change in the future. We can just do a line by line
> comparison of the gdb csr file against the csr-map.h file to verify
> that the csr names match. And it makes for a cleaner patch if the gdb
> csr file register numbering gets fixed in the future, in that case we
> can just delete this file and change a few lines to stop using this
> variable.
>
> This variable is only meant to be used in one file, the
> target/riscv/gdbstubs.c file.
I think it makes more sense to just define the variable in the
gdbstubs.c file then. Can you move it to patch 5?
Alistair
>
> Jim
>
