This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: Add and move fall-through comments in system-specific code
- From: "Gabriel F. T. Gomes" <gabriel at inconstante dot eti dot br>
- To: Joseph Myers <joseph at codesourcery dot com>
- Cc: <libc-alpha at sourceware dot org>
- Date: Mon, 25 Feb 2019 22:31:14 -0300
- Subject: Re: Add and move fall-through comments in system-specific code
- References: <alpine.DEB.2.21.1902181826140.12080@digraph.polyomino.org.uk>
On Mon, Feb 18 2019, Joseph Myers wrote:
>
> * sysdeps/i386/dl-machine.h (elf_machine_rela): Add fall-through
> comments.
> * sysdeps/m68k/m680x0/fpu/s_cexp_template.c (s(__cexp)): Likewise.
> * sysdeps/m68k/memcopy.h (WORD_COPY_FWD): Likewise.
> (WORD_COPY_BWD): Likewise.
> * sysdeps/mach/hurd/ioctl.c (__ioctl): Likewise.
> * sysdeps/powerpc/powerpc64/dl-machine.h (elf_machine_rela):
> Likewise.
> * sysdeps/s390/iso-8859-1_cp037_z900.c (TR_LOOP): Likewise.
> * sysdeps/mips/dl-machine.h (elf_machine_reloc): Move fall-through
> comment.
> * sysdeps/mips/dl-trampoline.c (__dl_runtime_resolve): Likewise.
Looks good to me.
> diff --git a/sysdeps/i386/dl-machine.h b/sysdeps/i386/dl-machine.h
> index 13cb03a7ab..1566d1282a 100644
> --- a/sysdeps/i386/dl-machine.h
> +++ b/sysdeps/i386/dl-machine.h
> @@ -522,6 +522,7 @@ elf_machine_rela (struct link_map *map, const Elf32_Rela *reloc,
> case R_386_SIZE32:
> /* Set to symbol size plus addend. */
> value = sym->st_size;
> + /* Fall through. */
> case R_386_GLOB_DAT:
> case R_386_JMP_SLOT:
> case R_386_32:
Looks good (size + addend).
> diff --git a/sysdeps/m68k/m680x0/fpu/s_cexp_template.c b/sysdeps/m68k/m680x0/fpu/s_cexp_template.c
> index d214f5925b..13befb2d29 100644
> --- a/sysdeps/m68k/m680x0/fpu/s_cexp_template.c
> +++ b/sysdeps/m68k/m680x0/fpu/s_cexp_template.c
> @@ -93,6 +93,7 @@ s(__cexp) (CFLOAT x)
> break;
> case 2:
> __real__ retval = -__real__ retval;
> + /* Fall through. */
Looks good (the -/- quadrant).
> case 3:
> __imag__ retval = -__imag__ retval;
> break;
> diff --git a/sysdeps/m68k/memcopy.h b/sysdeps/m68k/memcopy.h
> index 66c39649da..aa4a1ab651 100644
> --- a/sysdeps/m68k/memcopy.h
> +++ b/sysdeps/m68k/memcopy.h
> @@ -39,20 +39,28 @@
> do \
> { \
> ((op_t *) dst_bp)[0] = ((op_t *) src_bp)[0]; \
> + /* Fall through. */ \
> case 7: \
> ((op_t *) dst_bp)[1] = ((op_t *) src_bp)[1]; \
> + /* Fall through. */ \
> case 6: \
> ((op_t *) dst_bp)[2] = ((op_t *) src_bp)[2]; \
> + /* Fall through. */ \
> case 5: \
> ((op_t *) dst_bp)[3] = ((op_t *) src_bp)[3]; \
> + /* Fall through. */ \
> case 4: \
> ((op_t *) dst_bp)[4] = ((op_t *) src_bp)[4]; \
> + /* Fall through. */ \
> case 3: \
> ((op_t *) dst_bp)[5] = ((op_t *) src_bp)[5]; \
> + /* Fall through. */ \
> case 2: \
> ((op_t *) dst_bp)[6] = ((op_t *) src_bp)[6]; \
> + /* Fall through. */ \
> case 1: \
> ((op_t *) dst_bp)[7] = ((op_t *) src_bp)[7]; \
> + /* Fall through. */ \
> case 0: \
> src_bp += 32; \
> dst_bp += 32; \
Looks good (copy 1, 2, ... or 8 items).
> @@ -73,20 +81,28 @@
> do \
> { \
> *--__dst_ep = *--__src_ep; \
> + /* Fall through. */ \
> case 7: \
> *--__dst_ep = *--__src_ep; \
> + /* Fall through. */ \
> case 6: \
> *--__dst_ep = *--__src_ep; \
> + /* Fall through. */ \
> case 5: \
> *--__dst_ep = *--__src_ep; \
> + /* Fall through. */ \
> case 4: \
> *--__dst_ep = *--__src_ep; \
> + /* Fall through. */ \
> case 3: \
> *--__dst_ep = *--__src_ep; \
> + /* Fall through. */ \
> case 2: \
> *--__dst_ep = *--__src_ep; \
> + /* Fall through. */ \
> case 1: \
> *--__dst_ep = *--__src_ep; \
> + /* Fall through. */ \
> case 0: \
> __nblocks--; \
Likewise.
> } \
> diff --git a/sysdeps/mach/hurd/ioctl.c b/sysdeps/mach/hurd/ioctl.c
> index 625333110f..b0ad14a8e2 100644
> --- a/sysdeps/mach/hurd/ioctl.c
> +++ b/sysdeps/mach/hurd/ioctl.c
> @@ -177,6 +177,7 @@ __ioctl (int fd, unsigned long int request, ...)
> case MACH_SEND_INVALID_REPLY:
> case MACH_RCV_INVALID_NAME:
> __mig_dealloc_reply_port (m->msgh_local_port);
> + /* Fall through. */
> default:
> return err;
> }
Looks correct for an invalid path.
> @@ -318,6 +319,7 @@ __ioctl (int fd, unsigned long int request, ...)
> case EOPNOTSUPP:
> /* The server didn't understand the RPC. */
> err = ENOTTY;
> + /* Fall through. */
> default:
> return __hurd_fail (err);
> }
Likewise.
> diff --git a/sysdeps/mips/dl-machine.h b/sysdeps/mips/dl-machine.h
> index e82891fa3f..f9e7e90b41 100644
> --- a/sysdeps/mips/dl-machine.h
> +++ b/sysdeps/mips/dl-machine.h
> @@ -712,8 +712,8 @@ elf_machine_reloc (struct link_map *map, ElfW(Addr) r_info,
> it's totally unnecessary. */
> if (ELFW(R_SYM) (r_info) == 0)
> break;
> - /* Fall through. */
> #endif
> + /* Fall through. */
> default:
> _dl_reloc_bad_type (map, r_type, 0);
> break;
OK (comment placement).
> diff --git a/sysdeps/mips/dl-trampoline.c b/sysdeps/mips/dl-trampoline.c
> index 568c8a10ce..5a8cc7dc56 100644
> --- a/sysdeps/mips/dl-trampoline.c
> +++ b/sysdeps/mips/dl-trampoline.c
> @@ -166,8 +166,8 @@ __dl_runtime_resolve (ElfW(Word) sym_index,
>
> break;
> }
> - /* Fall through. */
> }
> + /* Fall through. */
> case 0:
> {
> /* We need to keep the scope around so do some locking. This is
Likewise.
> diff --git a/sysdeps/powerpc/powerpc64/dl-machine.h b/sysdeps/powerpc/powerpc64/dl-machine.h
> index bc8bd0230e..1d926e3dff 100644
> --- a/sysdeps/powerpc/powerpc64/dl-machine.h
> +++ b/sysdeps/powerpc/powerpc64/dl-machine.h
> @@ -902,6 +902,7 @@ elf_machine_rela (struct link_map *map,
> case R_PPC64_ADDR16_HI:
> if (dont_expect (value + 0x80000000 >= 0x100000000LL))
> _dl_reloc_overflow (map, "R_PPC64_ADDR16_HI", reloc_addr, refsym);
> + /* Fall through. */
> case R_PPC64_ADDR16_HIGH:
> *(Elf64_Half *) reloc_addr = PPC_HI (value);
> break;
OK. R_PPC64_ADDR16_HI reports overflows, whereas R_PPC64_ADDR16_HIGH
don't, as explained in the commit 7ec07d9a7b50.
> @@ -909,6 +910,7 @@ elf_machine_rela (struct link_map *map,
> case R_PPC64_ADDR16_HA:
> if (dont_expect (value + 0x80008000 >= 0x100000000LL))
> _dl_reloc_overflow (map, "R_PPC64_ADDR16_HA", reloc_addr, refsym);
> + /* Fall through. */
> case R_PPC64_ADDR16_HIGHA:
> *(Elf64_Half *) reloc_addr = PPC_HA (value);
> break;
Likewise, for *HA and *HIGHA.
> diff --git a/sysdeps/s390/iso-8859-1_cp037_z900.c b/sysdeps/s390/iso-8859-1_cp037_z900.c
> index b2d8f62570..2a373fe124 100644
> --- a/sysdeps/s390/iso-8859-1_cp037_z900.c
> +++ b/sysdeps/s390/iso-8859-1_cp037_z900.c
> @@ -227,12 +227,19 @@ __attribute__ ((aligned (8))) =
> switch (length) \
> { \
> case 7: outptr[6] = TABLE[inptr[6]]; \
> + /* Fall through. */ \
> case 6: outptr[5] = TABLE[inptr[5]]; \
> + /* Fall through. */ \
> case 5: outptr[4] = TABLE[inptr[4]]; \
> + /* Fall through. */ \
> case 4: outptr[3] = TABLE[inptr[3]]; \
> + /* Fall through. */ \
> case 3: outptr[2] = TABLE[inptr[2]]; \
> + /* Fall through. */ \
> case 2: outptr[1] = TABLE[inptr[1]]; \
> + /* Fall through. */ \
> case 1: outptr[0] = TABLE[inptr[0]]; \
> + /* Fall through. */ \
> case 0: break; \
> } \
> inptr += length; \
>
Looks good (proccess 1, 2, .. or 7 items).