* [PATCH V3 0/2] Fix build issues
@ 2020-05-18 21:53 Guoqing Jiang
2020-05-18 21:53 ` [PATCH V3 1/2] uuid.c: split uuid stuffs from util.c Guoqing Jiang
2020-05-18 21:53 ` [PATCH V3 2/2] restripe: fix ignoring return value of ‘read’ and lseek Guoqing Jiang
0 siblings, 2 replies; 5+ messages in thread
From: Guoqing Jiang @ 2020-05-18 21:53 UTC (permalink / raw)
To: jes; +Cc: linux-raid, Guoqing Jiang
Hi Jes,
A new version per your comment, let me know if I still missed something.
V3 Change:
1. check the return value of lseek
V2 Changes:
1. copy the whole license from util.c to uuid.c
2. check the return value of read correctly
Thanks,
Guoqing
Guoqing Jiang (2):
uuid.c: split uuid stuffs from util.c
restripe: fix ignoring return value of ‘read’ and lseek
Makefile | 6 +--
restripe.c | 12 +++++-
util.c | 87 -----------------------------------------
uuid.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 125 insertions(+), 92 deletions(-)
create mode 100644 uuid.c
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH V3 1/2] uuid.c: split uuid stuffs from util.c
2020-05-18 21:53 [PATCH V3 0/2] Fix build issues Guoqing Jiang
@ 2020-05-18 21:53 ` Guoqing Jiang
2020-05-19 0:11 ` Jes Sorensen
2020-05-18 21:53 ` [PATCH V3 2/2] restripe: fix ignoring return value of ‘read’ and lseek Guoqing Jiang
1 sibling, 1 reply; 5+ messages in thread
From: Guoqing Jiang @ 2020-05-18 21:53 UTC (permalink / raw)
To: jes; +Cc: linux-raid, Guoqing Jiang
Currently, 'make raid6check' is build broken since commit b06815989
("mdadm: load default sysfs attributes after assemblation").
/usr/bin/ld: sysfs.o: in function `sysfsline':
sysfs.c:(.text+0x2707): undefined reference to `parse_uuid'
/usr/bin/ld: sysfs.c:(.text+0x271a): undefined reference to `uuid_zero'
/usr/bin/ld: sysfs.c:(.text+0x2721): undefined reference to `uuid_zero'
Apparently, the compile of mdadm or raid6check are coupled with uuid
functions inside util.c. However, we can't just add util.o to CHECK_OBJS
which raid6check is needed, because it caused other worse problems.
So, let's introduce a uuid.c file which is indenpended file to fix the
problem, all the contents are splitted from util.c.
Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
---
Makefile | 6 +--
util.c | 87 ------------------------------------------
uuid.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 115 insertions(+), 90 deletions(-)
create mode 100644 uuid.c
diff --git a/Makefile b/Makefile
index a33319a..d13030c 100644
--- a/Makefile
+++ b/Makefile
@@ -139,7 +139,7 @@ else
ECHO=:
endif
-OBJS = mdadm.o config.o policy.o mdstat.o ReadMe.o util.o maps.o lib.o \
+OBJS = mdadm.o config.o policy.o mdstat.o ReadMe.o uuid.o util.o maps.o lib.o \
Manage.o Assemble.o Build.o \
Create.o Detail.o Examine.o Grow.o Monitor.o dlink.o Kill.o Query.o \
Incremental.o Dump.o \
@@ -148,13 +148,13 @@ OBJS = mdadm.o config.o policy.o mdstat.o ReadMe.o util.o maps.o lib.o \
restripe.o sysfs.o sha1.o mapfile.o crc32.o sg_io.o msg.o xmalloc.o \
platform-intel.o probe_roms.o crc32c.o
-CHECK_OBJS = restripe.o sysfs.o maps.o lib.o xmalloc.o dlink.o
+CHECK_OBJS = restripe.o uuid.o sysfs.o maps.o lib.o xmalloc.o dlink.o
SRCS = $(patsubst %.o,%.c,$(OBJS))
INCL = mdadm.h part.h bitmap.h
-MON_OBJS = mdmon.o monitor.o managemon.o util.o maps.o mdstat.o sysfs.o \
+MON_OBJS = mdmon.o monitor.o managemon.o uuid.o util.o maps.o mdstat.o sysfs.o \
policy.o lib.o \
Kill.o sg_io.o dlink.o ReadMe.o super-intel.o \
super-mbr.o super-gpt.o \
diff --git a/util.c b/util.c
index 07f9dc3..579dd42 100644
--- a/util.c
+++ b/util.c
@@ -306,43 +306,6 @@ int md_get_disk_info(int fd, struct mdu_disk_info_s *disk)
return ioctl(fd, GET_DISK_INFO, disk);
}
-/*
- * Parse a 128 bit uuid in 4 integers
- * format is 32 hexx nibbles with options :.<space> separator
- * If not exactly 32 hex digits are found, return 0
- * else return 1
- */
-int parse_uuid(char *str, int uuid[4])
-{
- int hit = 0; /* number of Hex digIT */
- int i;
- char c;
- for (i = 0; i < 4; i++)
- uuid[i] = 0;
-
- while ((c = *str++) != 0) {
- int n;
- if (c >= '0' && c <= '9')
- n = c-'0';
- else if (c >= 'a' && c <= 'f')
- n = 10 + c - 'a';
- else if (c >= 'A' && c <= 'F')
- n = 10 + c - 'A';
- else if (strchr(":. -", c))
- continue;
- else return 0;
-
- if (hit<32) {
- uuid[hit/8] <<= 4;
- uuid[hit/8] += n;
- }
- hit++;
- }
- if (hit == 32)
- return 1;
- return 0;
-}
-
int get_linux_version()
{
struct utsname name;
@@ -611,56 +574,6 @@ int enough(int level, int raid_disks, int layout, int clean, char *avail)
}
}
-const int uuid_zero[4] = { 0, 0, 0, 0 };
-
-int same_uuid(int a[4], int b[4], int swapuuid)
-{
- if (swapuuid) {
- /* parse uuids are hostendian.
- * uuid's from some superblocks are big-ending
- * if there is a difference, we need to swap..
- */
- unsigned char *ac = (unsigned char *)a;
- unsigned char *bc = (unsigned char *)b;
- int i;
- for (i = 0; i < 16; i += 4) {
- if (ac[i+0] != bc[i+3] ||
- ac[i+1] != bc[i+2] ||
- ac[i+2] != bc[i+1] ||
- ac[i+3] != bc[i+0])
- return 0;
- }
- return 1;
- } else {
- if (a[0]==b[0] &&
- a[1]==b[1] &&
- a[2]==b[2] &&
- a[3]==b[3])
- return 1;
- return 0;
- }
-}
-
-void copy_uuid(void *a, int b[4], int swapuuid)
-{
- if (swapuuid) {
- /* parse uuids are hostendian.
- * uuid's from some superblocks are big-ending
- * if there is a difference, we need to swap..
- */
- unsigned char *ac = (unsigned char *)a;
- unsigned char *bc = (unsigned char *)b;
- int i;
- for (i = 0; i < 16; i += 4) {
- ac[i+0] = bc[i+3];
- ac[i+1] = bc[i+2];
- ac[i+2] = bc[i+1];
- ac[i+3] = bc[i+0];
- }
- } else
- memcpy(a, b, 16);
-}
-
char *__fname_from_uuid(int id[4], int swap, char *buf, char sep)
{
int i, j;
diff --git a/uuid.c b/uuid.c
new file mode 100644
index 0000000..94b5abd
--- /dev/null
+++ b/uuid.c
@@ -0,0 +1,112 @@
+/*
+ * mdadm - manage Linux "md" devices aka RAID arrays.
+ *
+ * Copyright (C) 2001-2013 Neil Brown <neilb@suse.de>
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Author: Neil Brown
+ * Email: <neilb@suse.de>
+ */
+
+#include <string.h>
+
+const int uuid_zero[4] = { 0, 0, 0, 0 };
+
+int same_uuid(int a[4], int b[4], int swapuuid)
+{
+ if (swapuuid) {
+ /* parse uuids are hostendian.
+ * uuid's from some superblocks are big-ending
+ * if there is a difference, we need to swap..
+ */
+ unsigned char *ac = (unsigned char *)a;
+ unsigned char *bc = (unsigned char *)b;
+ int i;
+ for (i = 0; i < 16; i += 4) {
+ if (ac[i+0] != bc[i+3] ||
+ ac[i+1] != bc[i+2] ||
+ ac[i+2] != bc[i+1] ||
+ ac[i+3] != bc[i+0])
+ return 0;
+ }
+ return 1;
+ } else {
+ if (a[0]==b[0] &&
+ a[1]==b[1] &&
+ a[2]==b[2] &&
+ a[3]==b[3])
+ return 1;
+ return 0;
+ }
+}
+
+void copy_uuid(void *a, int b[4], int swapuuid)
+{
+ if (swapuuid) {
+ /* parse uuids are hostendian.
+ * uuid's from some superblocks are big-ending
+ * if there is a difference, we need to swap..
+ */
+ unsigned char *ac = (unsigned char *)a;
+ unsigned char *bc = (unsigned char *)b;
+ int i;
+ for (i = 0; i < 16; i += 4) {
+ ac[i+0] = bc[i+3];
+ ac[i+1] = bc[i+2];
+ ac[i+2] = bc[i+1];
+ ac[i+3] = bc[i+0];
+ }
+ } else
+ memcpy(a, b, 16);
+}
+
+/*
+ * Parse a 128 bit uuid in 4 integers
+ * format is 32 hexx nibbles with options :.<space> separator
+ * If not exactly 32 hex digits are found, return 0
+ * else return 1
+ */
+int parse_uuid(char *str, int uuid[4])
+{
+ int hit = 0; /* number of Hex digIT */
+ int i;
+ char c;
+ for (i = 0; i < 4; i++)
+ uuid[i] = 0;
+
+ while ((c = *str++) != 0) {
+ int n;
+ if (c >= '0' && c <= '9')
+ n = c-'0';
+ else if (c >= 'a' && c <= 'f')
+ n = 10 + c - 'a';
+ else if (c >= 'A' && c <= 'F')
+ n = 10 + c - 'A';
+ else if (strchr(":. -", c))
+ continue;
+ else return 0;
+
+ if (hit<32) {
+ uuid[hit/8] <<= 4;
+ uuid[hit/8] += n;
+ }
+ hit++;
+ }
+ if (hit == 32)
+ return 1;
+ return 0;
+}
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH V3 2/2] restripe: fix ignoring return value of ‘read’ and lseek
2020-05-18 21:53 [PATCH V3 0/2] Fix build issues Guoqing Jiang
2020-05-18 21:53 ` [PATCH V3 1/2] uuid.c: split uuid stuffs from util.c Guoqing Jiang
@ 2020-05-18 21:53 ` Guoqing Jiang
2020-05-19 0:26 ` Jes Sorensen
1 sibling, 1 reply; 5+ messages in thread
From: Guoqing Jiang @ 2020-05-18 21:53 UTC (permalink / raw)
To: jes; +Cc: linux-raid, Guoqing Jiang
Got below error when run "make everything".
restripe.c: In function ‘test_stripes’:
restripe.c:870:4: error: ignoring return value of ‘read’, declared with attribute warn_unused_result [-Werror=unused-result]
read(source[i], stripes[i], chunk_size);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Fix it by check the return value of ‘read’, and free memory
in the failure case.
And check the return value of lseek as well per Jes's comment.
Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
---
restripe.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/restripe.c b/restripe.c
index 31b07e8..86e1d00 100644
--- a/restripe.c
+++ b/restripe.c
@@ -866,8 +866,16 @@ int test_stripes(int *source, unsigned long long *offsets,
int disk;
for (i = 0 ; i < raid_disks ; i++) {
- lseek64(source[i], offsets[i]+start, 0);
- read(source[i], stripes[i], chunk_size);
+ if ((lseek64(source[i], offsets[i]+start, 0) < 0) ||
+ (read(source[i], stripes[i], chunk_size) !=
+ chunk_size)) {
+ free(q);
+ free(p);
+ free(blocks);
+ free(stripes);
+ free(stripe_buf);
+ return -1;
+ }
}
for (i = 0 ; i < data_disks ; i++) {
int disk = geo_map(i, start/chunk_size, raid_disks,
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH V3 1/2] uuid.c: split uuid stuffs from util.c
2020-05-18 21:53 ` [PATCH V3 1/2] uuid.c: split uuid stuffs from util.c Guoqing Jiang
@ 2020-05-19 0:11 ` Jes Sorensen
0 siblings, 0 replies; 5+ messages in thread
From: Jes Sorensen @ 2020-05-19 0:11 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: linux-raid
On 5/18/20 5:53 PM, Guoqing Jiang wrote:
> Currently, 'make raid6check' is build broken since commit b06815989
> ("mdadm: load default sysfs attributes after assemblation").
>
> /usr/bin/ld: sysfs.o: in function `sysfsline':
> sysfs.c:(.text+0x2707): undefined reference to `parse_uuid'
> /usr/bin/ld: sysfs.c:(.text+0x271a): undefined reference to `uuid_zero'
> /usr/bin/ld: sysfs.c:(.text+0x2721): undefined reference to `uuid_zero'
>
> Apparently, the compile of mdadm or raid6check are coupled with uuid
> functions inside util.c. However, we can't just add util.o to CHECK_OBJS
> which raid6check is needed, because it caused other worse problems.
>
> So, let's introduce a uuid.c file which is indenpended file to fix the
> problem, all the contents are splitted from util.c.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
> ---
> Makefile | 6 +--
> util.c | 87 ------------------------------------------
> uuid.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 115 insertions(+), 90 deletions(-)
> create mode 100644 uuid.c
Applied!
Thanks,
Jes
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V3 2/2] restripe: fix ignoring return value of ‘read’ and lseek
2020-05-18 21:53 ` [PATCH V3 2/2] restripe: fix ignoring return value of ‘read’ and lseek Guoqing Jiang
@ 2020-05-19 0:26 ` Jes Sorensen
0 siblings, 0 replies; 5+ messages in thread
From: Jes Sorensen @ 2020-05-19 0:26 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: linux-raid
On 5/18/20 5:53 PM, Guoqing Jiang wrote:
> Got below error when run "make everything".
>
> restripe.c: In function ‘test_stripes’:
> restripe.c:870:4: error: ignoring return value of ‘read’, declared with attribute warn_unused_result [-Werror=unused-result]
> read(source[i], stripes[i], chunk_size);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Fix it by check the return value of ‘read’, and free memory
> in the failure case.
>
> And check the return value of lseek as well per Jes's comment.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
> ---
> restripe.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/restripe.c b/restripe.c
> index 31b07e8..86e1d00 100644
> --- a/restripe.c
> +++ b/restripe.c
> @@ -866,8 +866,16 @@ int test_stripes(int *source, unsigned long long *offsets,
> int disk;
>
> for (i = 0 ; i < raid_disks ; i++) {
> - lseek64(source[i], offsets[i]+start, 0);
> - read(source[i], stripes[i], chunk_size);
> + if ((lseek64(source[i], offsets[i]+start, 0) < 0) ||
> + (read(source[i], stripes[i], chunk_size) !=
> + chunk_size)) {
> + free(q);
> + free(p);
> + free(blocks);
> + free(stripes);
> + free(stripe_buf);
> + return -1;
> + }
> }
> for (i = 0 ; i < data_disks ; i++) {
> int disk = geo_map(i, start/chunk_size, raid_disks,
>
This is not the prettiest solution, would have been nicer to introduce a
cleanup handler at the end and just jumping to it.
That said, I have applied it since restripe.c is pretty ugly across the
board.
Thanks,
Jes
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-05-19 0:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-18 21:53 [PATCH V3 0/2] Fix build issues Guoqing Jiang
2020-05-18 21:53 ` [PATCH V3 1/2] uuid.c: split uuid stuffs from util.c Guoqing Jiang
2020-05-19 0:11 ` Jes Sorensen
2020-05-18 21:53 ` [PATCH V3 2/2] restripe: fix ignoring return value of ‘read’ and lseek Guoqing Jiang
2020-05-19 0:26 ` Jes Sorensen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.