[PATCH] Add -Wmissing-prototypes and fix all findings

[Jan Kiszka]

From: Jan Kiszka <jan.k...@siemens.com>

No functional changes, just cleanups.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
Makefile.am | 1 +
tools/bg_utils.c | 12 ++++++------
tools/ebgpart.c | 31 ++++++++++++++++---------------
tools/ebgpart.h | 2 +-
4 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 7745bb1..6038f30 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -24,6 +24,7 @@ AM_CPPFLAGS = -I$(top_srcdir)/include -I$(top_srcdir) -include config.h

AM_CFLAGS = \
-Wno-unused-parameter \
+ -Wmissing-prototypes \
-fshort-wchar \
-DHAVE_ENDIAN_H \
-D_GNU_SOURCE
diff --git a/tools/bg_utils.c b/tools/bg_utils.c
index 026abb9..1a5dd76 100644
--- a/tools/bg_utils.c
+++ b/tools/bg_utils.c
@@ -89,7 +89,7 @@ static char *get_mountpoint(char *devpath)
return NULL;
}

-bool mount_partition(CONFIG_PART *cfgpart)
+static bool mount_partition(CONFIG_PART *cfgpart)
{
char tmpdir_template[256];
char *mountpoint;
@@ -121,7 +121,7 @@ bool mount_partition(CONFIG_PART *cfgpart)
return true;
}

-void unmount_partition(CONFIG_PART *cfgpart)
+static void unmount_partition(CONFIG_PART *cfgpart)
{
if (!cfgpart) {
return;
@@ -159,7 +159,7 @@ static FILE *open_config_file(CONFIG_PART *cfgpart, char *mode)
return config;
}

-bool probe_config_file(CONFIG_PART *cfgpart)
+static bool probe_config_file(CONFIG_PART *cfgpart)
{
bool do_unmount = false;
if (!cfgpart) {
@@ -206,7 +206,7 @@ bool probe_config_file(CONFIG_PART *cfgpart)
return false;
}

-bool probe_config_partitions(CONFIG_PART *cfgpart)
+static bool probe_config_partitions(CONFIG_PART *cfgpart)
{
PedDevice *dev = NULL;
char devpath[4096];
@@ -273,7 +273,7 @@ bool probe_config_partitions(CONFIG_PART *cfgpart)
return true;
}

-bool read_env(CONFIG_PART *part, BG_ENVDATA *env)
+static bool read_env(CONFIG_PART *part, BG_ENVDATA *env)
{
if (!part) {
return false;
@@ -310,7 +310,7 @@ bool read_env(CONFIG_PART *part, BG_ENVDATA *env)
return result;
}

-bool write_env(CONFIG_PART *part, BG_ENVDATA *env)
+static bool write_env(CONFIG_PART *part, BG_ENVDATA *env)
{
if (!part) {
return false;
diff --git a/tools/ebgpart.c b/tools/ebgpart.c
index 878cc5e..d0997bd 100644
--- a/tools/ebgpart.c
+++ b/tools/ebgpart.c
@@ -28,7 +28,7 @@ void ebgpart_beverbose(bool v)
verbosity = v;
}

-void add_block_dev(PedDevice *dev)
+static void add_block_dev(PedDevice *dev)
{
if (!first_device) {
first_device = dev;
@@ -41,7 +41,7 @@ void add_block_dev(PedDevice *dev)
d->next = dev;
}

-char *GUID_to_str(uint8_t *g)
+static char *GUID_to_str(uint8_t *g)
{
snprintf(buffer, 37, "%02X%02X%02X%02X-%02X%02X-%02X%02X-%02X%02X-%02X%"
"02X%02X%02X%02X%02X",
@@ -50,7 +50,7 @@ char *GUID_to_str(uint8_t *g)
return buffer;
}

-char *type_to_name(char t)
+static char *type_to_name(char t)
{
switch (t) {
case MBR_TYPE_FAT12:
@@ -69,8 +69,8 @@ char *type_to_name(char t)
return "not supported";
}

-bool check_GPT_FAT_entry(int fd, struct EFIpartitionentry *e,
- PedFileSystemType *pfst, uint32_t i)
+static bool check_GPT_FAT_entry(int fd, struct EFIpartitionentry *e,
+ PedFileSystemType *pfst, uint32_t i)
{
if (strcmp(GPT_PARTITION_GUID_FAT_NTFS, GUID_to_str(e->type_GUID)) !=
0 &&
@@ -131,7 +131,8 @@ bool check_GPT_FAT_entry(int fd, struct EFIpartitionentry *e,
return true;
}

-void read_GPT_entries(int fd, uint64_t table_LBA, uint32_t num, PedDevice *dev)
+static void read_GPT_entries(int fd, uint64_t table_LBA, uint32_t num,
+ PedDevice *dev)
{
off64_t offset;
struct EFIpartitionentry e;
@@ -185,9 +186,9 @@ void read_GPT_entries(int fd, uint64_t table_LBA, uint32_t num, PedDevice *dev)
}
}

-void scanLogicalVolumes(int fd, off64_t extended_start_LBA,
- struct Masterbootrecord *ebr, int i,
- PedPartition *partition, int lognum)
+static void scanLogicalVolumes(int fd, off64_t extended_start_LBA,
+ struct Masterbootrecord *ebr, int i,
+ PedPartition *partition, int lognum)
{
struct Masterbootrecord next_ebr;
PedFileSystemType *pfst;
@@ -246,7 +247,7 @@ scl_out_of_mem:
if (partition->next) free(partition->next);
}

-bool check_partition_table(PedDevice *dev)
+static bool check_partition_table(PedDevice *dev)
{
int fd;
struct Masterbootrecord mbr;
@@ -346,8 +347,8 @@ bool check_partition_table(PedDevice *dev)
return true;
}

-int scan_devdir(unsigned int fmajor, unsigned int fminor, char *fullname,
- unsigned int maxlen)
+static int scan_devdir(unsigned int fmajor, unsigned int fminor, char *fullname,
+ unsigned int maxlen)
{
int result = -1;

@@ -398,7 +399,7 @@ static int get_major_minor(char *filename, unsigned int *major, unsigned int *mi
return 0;
}

-void ped_device_probe_all()
+void ped_device_probe_all(void)
{
struct dirent *sysblockfile;
char fullname[DEV_FILENAME_LEN];
@@ -457,7 +458,7 @@ void ped_device_probe_all()
closedir(sysblockdir);
}

-void ped_partition_destroy(PedPartition *p)
+static void ped_partition_destroy(PedPartition *p)
{
if (!p) return;
if (!p->fs_type) goto fs_type_Null;
@@ -467,7 +468,7 @@ fs_type_Null:
free(p);
}

-void ped_device_destroy(PedDevice *d)
+static void ped_device_destroy(PedDevice *d)
{
if (!d) return;
if (d->model) free(d->model);
diff --git a/tools/ebgpart.h b/tools/ebgpart.h
index 918602e..097bf8f 100644
--- a/tools/ebgpart.h
+++ b/tools/ebgpart.h
@@ -131,7 +131,7 @@ typedef struct _PedDisk {
PedPartition *part_list;
} PedDisk;

-void ped_device_probe_all();
+void ped_device_probe_all(void);
PedDevice *ped_device_get_next(const PedDevice *dev);
PedDisk *ped_disk_new(const PedDevice *dev);
PedPartition *ped_disk_next_partition(const PedDisk *pd,
--
2.12.3

[Andreas Reichel]

On Fri, Jul 21, 2017 at 06:37:42PM +0200, [ext] Jan Kiszka wrote:
> From: Jan Kiszka <jan.k...@siemens.com>
>
> No functional changes, just cleanups.

If you define all functions as static, this IS a functional change.
Because they are not found by the test linkage anymore, which breaks
all tests.

Please revert that.

Kind regards
Andreas

➜ tools git:(next) ✗ cd tests
➜ tests git:(next) ✗ make
cc -I/home/projects/efibootguard/efibootguard/tools/tests/ -I/home/projects/efibootguard/efibootguard/tools/tests//.. -I/home/projects/efibootguard/efibootguard/tools/tests//../../include -I/home/projects/efibootguard/efibootguard/tools/tests//../../swupdate-adapter -std=gnu99 -g -fshort-wchar -DHAVE_ENDIAN_H -D_GNU_SOURCE -c test_partitions.c -o test_partitions.o
cc -I/home/projects/efibootguard/efibootguard/tools/tests/ -I/home/projects/efibootguard/efibootguard/tools/tests//.. -I/home/projects/efibootguard/efibootguard/tools/tests//../../include -I/home/projects/efibootguard/efibootguard/tools/tests//../../swupdate-adapter -std=gnu99 -g -fshort-wchar -DHAVE_ENDIAN_H -D_GNU_SOURCE -c ../bg_utils.c -o bg_utils.o
cc -I/home/projects/efibootguard/efibootguard/tools/tests/ -I/home/projects/efibootguard/efibootguard/tools/tests//.. -I/home/projects/efibootguard/efibootguard/tools/tests//../../include -I/home/projects/efibootguard/efibootguard/tools/tests//../../swupdate-adapter -std=gnu99 -g -fshort-wchar -DHAVE_ENDIAN_H -D_GNU_SOURCE -c ../ebgpart.c -o ebgpart.o
objcopy --weaken-symbol probe_config_file bg_utils.o
objcopy --weaken-symbol ped_device_probe_all ebgpart.o
objcopy --weaken-symbol ped_device_get_next ebgpart.o
objcopy --weaken-symbol ped_disk_next_partition ebgpart.o
cc test_partitions.o bg_utils.o ebgpart.o -o test_partitions -L../.. -L../../swupdate-adapter -lcmocka -lebgenv -lz
test_partitions.o: In function `test_partition_count':
/home/projects/efibootguard/efibootguard/tools/tests/test_partitions.c:103: undefined reference to `probe_config_partitions'
/home/projects/efibootguard/efibootguard/tools/tests/test_partitions.c:111: undefined reference to `probe_config_partitions'
/home/projects/efibootguard/efibootguard/tools/tests/test_partitions.c:119: undefined reference to `probe_config_partitions'
/home/projects/efibootguard/efibootguard/tools/tests/test_partitions.c:127: undefined reference to `probe_config_partitions'
test_partitions.o: In function `test_config_file_existence':
/home/projects/efibootguard/efibootguard/tools/tests/test_partitions.c:143: undefined reference to `probe_config_partitions'
test_partitions.o:/home/projects/efibootguard/efibootguard/tools/tests/test_partitions.c:152: more undefined references to `probe_config_partitions' follow
collect2: error: ld returned 1 exit status
make: *** [Makefile:97: test_partitions.target] Error 1

> --
> You received this message because you are subscribed to the Google Groups "EFI Boot Guard" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to efibootguard-d...@googlegroups.com.
> To post to this group, send email to efibootg...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/efibootguard-dev/803b3ce1-439c-ede9-d663-42a4816db2fe%40siemens.com.
> For more options, visit https://groups.google.com/d/optout.

--
Andreas Reichel
Dipl.-Phys. (Univ.)
Software Consultant

Andreas...@tngtech.com, +49-174-3180074
TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterfoehring
Geschaeftsfuehrer: Henrik Klagges, Christoph Stock, Dr. Robert Dahlke
Sitz: Unterfoehring * Amtsgericht Muenchen * HRB 135082

[Jan Kiszka]

On 2017-07-24 10:58, Andreas Reichel wrote:
> On Fri, Jul 21, 2017 at 06:37:42PM +0200, [ext] Jan Kiszka wrote:
>> From: Jan Kiszka <jan.k...@siemens.com>
>>
>> No functional changes, just cleanups.
>
> If you define all functions as static, this IS a functional change.
> Because they are not found by the test linkage anymore, which breaks
> all tests.

OK, that's a bug in - of the test cases. Make them part of the regular
build, otherwise this will repeat over and over again.

Jan

--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

[Jan Kiszka]

From: Jan Kiszka <jan.k...@siemens.com>

Unfortunately, the current test setup requires to keep a number of
functions public in order to attach to them during unit tests. Move
their prototypes into a separate header that does not expose them
widely.

No functional changes, just cleanups.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---

I'm looking forward to patches the integrate the tests into the build.
Otherwise, they will bit-rot.

Makefile.am | 1 +
tools/bg_utils.c | 5 +++--

tools/ebgpart.c | 31 ++++++++++++++++---------------
tools/ebgpart.h | 2 +-

tools/test-interface.h | 24 ++++++++++++++++++++++++
tools/tests/test_environment.c | 4 +---
tools/tests/test_partitions.c | 3 +--
7 files changed, 47 insertions(+), 23 deletions(-)
create mode 100644 tools/test-interface.h

diff --git a/Makefile.am b/Makefile.am
index 7745bb1..6038f30 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -24,6 +24,7 @@ AM_CPPFLAGS = -I$(top_srcdir)/include -I$(top_srcdir) -include config.h

AM_CFLAGS = \
-Wno-unused-parameter \
+ -Wmissing-prototypes \
-fshort-wchar \
-DHAVE_ENDIAN_H \
-D_GNU_SOURCE
diff --git a/tools/bg_utils.c b/tools/bg_utils.c

index 026abb9..496e2c8 100644
--- a/tools/bg_utils.c
+++ b/tools/bg_utils.c
@@ -11,6 +11,7 @@
*/

#include "bg_utils.h"
+#include "test-interface.h"

const char *tmp_mnt_dir = "/tmp/mnt-XXXXXX";

@@ -89,7 +90,7 @@ static char *get_mountpoint(char *devpath)

return NULL;
}

-bool mount_partition(CONFIG_PART *cfgpart)
+static bool mount_partition(CONFIG_PART *cfgpart)
{
char tmpdir_template[256];
char *mountpoint;

@@ -121,7 +122,7 @@ bool mount_partition(CONFIG_PART *cfgpart)

return true;
}

-void unmount_partition(CONFIG_PART *cfgpart)
+static void unmount_partition(CONFIG_PART *cfgpart)
{
if (!cfgpart) {
return;

diff --git a/tools/test-interface.h b/tools/test-interface.h
new file mode 100644
index 0000000..d0bf6d4
--- /dev/null
+++ b/tools/test-interface.h
@@ -0,0 +1,24 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2017
+ *
+ * Authors:
+ * Jan Kiszka <jan.k...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef __TEST_INTERFACE_H__
+#define __TEST_INTERFACE_H__
+
+#include "bg_utils.h"
+
+bool read_env(CONFIG_PART *part, BG_ENVDATA *env);
+bool write_env(CONFIG_PART *part, BG_ENVDATA *env);
+
+bool probe_config_file(CONFIG_PART *cfgpart);
+bool probe_config_partitions(CONFIG_PART *cfgparts);
+
+#endif // __TEST_INTERFACE_H__
diff --git a/tools/tests/test_environment.c b/tools/tests/test_environment.c
index 7fa4061..fe9d50e 100644
--- a/tools/tests/test_environment.c
+++ b/tools/tests/test_environment.c
@@ -18,6 +18,7 @@
#include <cmocka.h>
#include <string.h>
#include "bg_utils.h"
+#include "test-interface.h"

/* Mock functions from libparted */

@@ -59,9 +60,6 @@ size_t fwrite(const void *ptr, size_t size, size_t count, FILE *stream)
return mock_type(size_t);
}

-extern bool read_env(CONFIG_PART *part, BG_ENVDATA *env);
-extern bool write_env(CONFIG_PART *part, BG_ENVDATA *env);
-
static void test_configfile_read(void **state)
{
CONFIG_PART part;
diff --git a/tools/tests/test_partitions.c b/tools/tests/test_partitions.c
index bb2bff3..6781b79 100644
--- a/tools/tests/test_partitions.c
+++ b/tools/tests/test_partitions.c
@@ -17,6 +17,7 @@
#include <setjmp.h>
#include <cmocka.h>
#include "bg_utils.h"
+#include "test-interface.h"

static PedDevice ped_devices[32] = {0};
static int num_simulated_devices = 2;
@@ -30,8 +31,6 @@ static const char *const fsname = "fat16";
static char *fakemodel = "Mocked Disk Drive";
static char *fakedevice = "/dev/nobrain";

-extern bool probe_config_partitions(CONFIG_PART *cfgparts);
-
/* Mock functions from libparted */
void ped_device_probe_all()
{