files-backend.c: fix build error on Solaris
diff mbox series

Message ID 20181125045816.12185-1-pclouds@gmail.com
State New
Headers show
Series
  • files-backend.c: fix build error on Solaris
Related show

Commit Message

Duy Nguyen Nov. 25, 2018, 4:58 a.m. UTC
This function files_reflog_path returns void, which usually means
"return;" not returning "void value" from another function.

Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 refs/files-backend.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Carlo Marcelo Arenas Belón Nov. 25, 2018, 10:19 a.m. UTC | #1
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>

clang with -Wpedantic also catch this (at least with Apple LLVM
version 10.0.0); recent versions of gcc also include that flag and at
least 8.2.0 shows a warning for it, so it might be worth adding it to
developer mode (maybe under the pedantic DEVOPTS), would this be
something I should be adding?, what are the expectations around
warnings and compilers?

Carlo
Duy Nguyen Nov. 25, 2018, 10:40 a.m. UTC | #2
On Sun, Nov 25, 2018 at 11:19 AM Carlo Arenas <carenas@gmail.com> wrote:
>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
>
> clang with -Wpedantic also catch this (at least with Apple LLVM
> version 10.0.0); recent versions of gcc also include that flag and at
> least 8.2.0 shows a warning for it, so it might be worth adding it to
> developer mode (maybe under the pedantic DEVOPTS), would this be
> something I should be adding?, what are the expectations around
> warnings and compilers?

make DEVELOPER=1 DEVOPTS=pedantic (with either gcc 7.3.0 or 8.2.0) can
already catch this. I have no comment if this pedantic should be part
of default DEVELOPER=1. But at least in controlled environments like
Travis, we could turn it on to catch more.
Junio C Hamano Nov. 26, 2018, 4:44 a.m. UTC | #3
Carlo Arenas <carenas@gmail.com> writes:

> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>

Do you mean Tested-by: (meaning, you actually saw the breakage with
SunCC without the patch and also saw the patch fixed the breakage)?

> clang with -Wpedantic also catch this (at least with Apple LLVM
> version 10.0.0); recent versions of gcc also include that flag and at
> least 8.2.0 shows a warning for it, so it might be worth adding it to
> developer mode (maybe under the pedantic DEVOPTS).

Nice to know.

> ..., would this be something I should be adding?, what are the expectations around
> warnings and compilers?

It may be a worthwhile thing to discuss, and as a discussion
starter, a patch would help ;-).

Patch
diff mbox series

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 9183875dad..dd8abe9185 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -180,7 +180,8 @@  static void files_reflog_path(struct files_ref_store *refs,
 		break;
 	case REF_TYPE_OTHER_PSEUDOREF:
 	case REF_TYPE_MAIN_PSEUDOREF:
-		return files_reflog_path_other_worktrees(refs, sb, refname);
+		files_reflog_path_other_worktrees(refs, sb, refname);
+		break;
 	case REF_TYPE_NORMAL:
 		strbuf_addf(sb, "%s/logs/%s", refs->gitcommondir, refname);
 		break;