| Message ID | 1513030760-26245-2-git-send-email-walling@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11.12.2017 23:19, Collin L. Walling wrote: > Moved: > memcmp from bootmap.h to libc.h (renamed from _memcmp) > strlen from sclp.c to libc.h (renamed from _strlen) > > Added C standard functions: > isdigit > atoi > > Added non-C standard function: > itostr > > Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> > Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> > --- > pc-bios/s390-ccw/Makefile | 2 +- > pc-bios/s390-ccw/bootmap.c | 4 +-- > pc-bios/s390-ccw/bootmap.h | 16 +--------- > pc-bios/s390-ccw/libc.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++ > pc-bios/s390-ccw/libc.h | 31 +++++++++++++++++++ > pc-bios/s390-ccw/main.c | 17 +---------- > pc-bios/s390-ccw/sclp.c | 10 +------ > 7 files changed, 112 insertions(+), 43 deletions(-) > create mode 100644 pc-bios/s390-ccw/libc.c [...] > + > +/** > + * itostr: > + * @num: the integer to be converted. > + * @str: a pointer to a string to store the conversion. > + * @len: the length of the passed string. > + * > + * Given an integer @num, convert it to a string. The string @str must be > + * allocated beforehand. The resulting string will be null terminated and > + * returned. > + * > + * Returns: the string @str of the converted integer @num. > + */ > +char *itostr(int num, char *str, size_t len) > +{ > + long num_len = 1; > + int tmp = num; > + int i; > + > + /* Count length of num */ > + while ((tmp /= 10) > 0) { > + num_len++; > + } > + > + /* Check if we have enough space for num and null */ > + if (len < num_len) { > + return 0; > + } I'm afraid, but I think you've got an off-by-one bug in this code. In patch 5, you're using this function like this: char tmp[4]; sclp_print(itostr(entries, tmp, sizeof(tmp))); That means if entries is >= 1000 for example, num_len is 4 ... > + /* Convert int to string */ > + for (i = num_len - 1; i >= 0; i--) { > + str[i] = num % 10 + '0'; > + num /= 10; > + } > + > + str[num_len] = '\0'; ... and then you run into a buffer overflow here. > + return str; > +} Maybe it would also make more sense to panic() instead of "return 0" since you don't check the return value in patch 5 ? Thomas
On 12/18/2017 08:06 AM, Thomas Huth wrote: > On 11.12.2017 23:19, Collin L. Walling wrote: >> Moved: >> memcmp from bootmap.h to libc.h (renamed from _memcmp) >> strlen from sclp.c to libc.h (renamed from _strlen) >> >> Added C standard functions: >> isdigit >> atoi >> >> Added non-C standard function: >> itostr >> >> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> >> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> >> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> >> --- >> pc-bios/s390-ccw/Makefile | 2 +- >> pc-bios/s390-ccw/bootmap.c | 4 +-- >> pc-bios/s390-ccw/bootmap.h | 16 +--------- >> pc-bios/s390-ccw/libc.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++ >> pc-bios/s390-ccw/libc.h | 31 +++++++++++++++++++ >> pc-bios/s390-ccw/main.c | 17 +---------- >> pc-bios/s390-ccw/sclp.c | 10 +------ >> 7 files changed, 112 insertions(+), 43 deletions(-) >> create mode 100644 pc-bios/s390-ccw/libc.c > [...] >> + >> +/** >> + * itostr: >> + * @num: the integer to be converted. >> + * @str: a pointer to a string to store the conversion. >> + * @len: the length of the passed string. >> + * >> + * Given an integer @num, convert it to a string. The string @str must be >> + * allocated beforehand. The resulting string will be null terminated and >> + * returned. >> + * >> + * Returns: the string @str of the converted integer @num. >> + */ >> +char *itostr(int num, char *str, size_t len) >> +{ >> + long num_len = 1; >> + int tmp = num; >> + int i; >> + >> + /* Count length of num */ >> + while ((tmp /= 10) > 0) { >> + num_len++; >> + } >> + >> + /* Check if we have enough space for num and null */ >> + if (len < num_len) { >> + return 0; >> + } > I'm afraid, but I think you've got an off-by-one bug in this code. > > In patch 5, you're using this function like this: > > char tmp[4]; > > sclp_print(itostr(entries, tmp, sizeof(tmp))); > > That means if entries is >= 1000 for example, num_len is 4 ... > >> + /* Convert int to string */ >> + for (i = num_len - 1; i >= 0; i--) { >> + str[i] = num % 10 + '0'; >> + num /= 10; >> + } >> + >> + str[num_len] = '\0'; > ... and then you run into a buffer overflow here. Doh, you're correct. I forgot to put a "<=" in the len / num_len check. That should fix things up. Thanks for catching that. > >> + return str; >> +} > Maybe it would also make more sense to panic() instead of "return 0" > since you don't check the return value in patch 5 ? I'm a bit conflicted about doing something like that. I'm not sure if there's any kind of guideline we want to follow for defining functions in libc. I see one of two possibilities: a. define these functions as "libc-like" as possible, and use them as if they were regular standard libc functions or b. change up these functions to better fit their use cases in pc-bios/s390-ccw Does that make sense? What do you think? > > Thomas >
On 18.12.2017 17:16, Collin L. Walling wrote: > On 12/18/2017 08:06 AM, Thomas Huth wrote: >> On 11.12.2017 23:19, Collin L. Walling wrote: >>> Moved: >>> memcmp from bootmap.h to libc.h (renamed from _memcmp) >>> strlen from sclp.c to libc.h (renamed from _strlen) >>> >>> Added C standard functions: >>> isdigit >>> atoi >>> >>> Added non-C standard function: >>> itostr >>> >>> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> >>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> >>> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> >>> --- >>> pc-bios/s390-ccw/Makefile | 2 +- >>> pc-bios/s390-ccw/bootmap.c | 4 +-- >>> pc-bios/s390-ccw/bootmap.h | 16 +--------- >>> pc-bios/s390-ccw/libc.c | 75 >>> ++++++++++++++++++++++++++++++++++++++++++++++ >>> pc-bios/s390-ccw/libc.h | 31 +++++++++++++++++++ >>> pc-bios/s390-ccw/main.c | 17 +---------- >>> pc-bios/s390-ccw/sclp.c | 10 +------ >>> 7 files changed, 112 insertions(+), 43 deletions(-) >>> create mode 100644 pc-bios/s390-ccw/libc.c >> [...] >>> + >>> +/** >>> + * itostr: >>> + * @num: the integer to be converted. >>> + * @str: a pointer to a string to store the conversion. >>> + * @len: the length of the passed string. >>> + * >>> + * Given an integer @num, convert it to a string. The string @str >>> must be >>> + * allocated beforehand. The resulting string will be null >>> terminated and >>> + * returned. >>> + * >>> + * Returns: the string @str of the converted integer @num. >>> + */ >>> +char *itostr(int num, char *str, size_t len) >>> +{ >>> + long num_len = 1; >>> + int tmp = num; >>> + int i; >>> + >>> + /* Count length of num */ >>> + while ((tmp /= 10) > 0) { >>> + num_len++; >>> + } >>> + >>> + /* Check if we have enough space for num and null */ >>> + if (len < num_len) { >>> + return 0; >>> + } >> I'm afraid, but I think you've got an off-by-one bug in this code. >> >> In patch 5, you're using this function like this: >> >> char tmp[4]; >> >> sclp_print(itostr(entries, tmp, sizeof(tmp))); >> >> That means if entries is >= 1000 for example, num_len is 4 ... >> >>> + /* Convert int to string */ >>> + for (i = num_len - 1; i >= 0; i--) { >>> + str[i] = num % 10 + '0'; >>> + num /= 10; >>> + } >>> + >>> + str[num_len] = '\0'; >> ... and then you run into a buffer overflow here. > > > Doh, you're correct. I forgot to put a "<=" in the len / num_len check. > That should fix things up. Thanks for catching that. > > >> >>> + return str; >>> +} >> Maybe it would also make more sense to panic() instead of "return 0" >> since you don't check the return value in patch 5 ? > > > I'm a bit conflicted about doing something like that. I'm not sure if > there's any kind > of guideline we want to follow for defining functions in libc. > > I see one of two possibilities: > > a. define these functions as "libc-like" as possible, and use them as > if they were > regular standard libc functions > > or > > b. change up these functions to better fit their use cases in > pc-bios/s390-ccw > > Does that make sense? What do you think? Keeping them libc-like likely makes sense ... but could we somehow also make sure that we're not running into unexpected errors when using them? Something like "IPL_assert(entries < 1000, ...)" before calling the functions in patch 5? Thomas
On 12/19/2017 02:31 AM, Thomas Huth wrote: > On 18.12.2017 17:16, Collin L. Walling wrote: >> On 12/18/2017 08:06 AM, Thomas Huth wrote: >>> On 11.12.2017 23:19, Collin L. Walling wrote: >>>> Moved: >>>> memcmp from bootmap.h to libc.h (renamed from _memcmp) >>>> strlen from sclp.c to libc.h (renamed from _strlen) >>>> >>>> Added C standard functions: >>>> isdigit >>>> atoi >>>> >>>> Added non-C standard function: >>>> itostr >>>> >>>> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> >>>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> >>>> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> >>>> --- >>>> pc-bios/s390-ccw/Makefile | 2 +- >>>> pc-bios/s390-ccw/bootmap.c | 4 +-- >>>> pc-bios/s390-ccw/bootmap.h | 16 +--------- >>>> pc-bios/s390-ccw/libc.c | 75 >>>> ++++++++++++++++++++++++++++++++++++++++++++++ >>>> pc-bios/s390-ccw/libc.h | 31 +++++++++++++++++++ >>>> pc-bios/s390-ccw/main.c | 17 +---------- >>>> pc-bios/s390-ccw/sclp.c | 10 +------ >>>> 7 files changed, 112 insertions(+), 43 deletions(-) >>>> create mode 100644 pc-bios/s390-ccw/libc.c >>> [...] >>>> + >>>> +/** >>>> + * itostr: >>>> + * @num: the integer to be converted. >>>> + * @str: a pointer to a string to store the conversion. >>>> + * @len: the length of the passed string. >>>> + * >>>> + * Given an integer @num, convert it to a string. The string @str >>>> must be >>>> + * allocated beforehand. The resulting string will be null >>>> terminated and >>>> + * returned. >>>> + * >>>> + * Returns: the string @str of the converted integer @num. >>>> + */ >>>> +char *itostr(int num, char *str, size_t len) >>>> +{ >>>> + long num_len = 1; >>>> + int tmp = num; >>>> + int i; >>>> + >>>> + /* Count length of num */ >>>> + while ((tmp /= 10) > 0) { >>>> + num_len++; >>>> + } >>>> + >>>> + /* Check if we have enough space for num and null */ >>>> + if (len < num_len) { >>>> + return 0; >>>> + } >>> I'm afraid, but I think you've got an off-by-one bug in this code. >>> >>> In patch 5, you're using this function like this: >>> >>> char tmp[4]; >>> >>> sclp_print(itostr(entries, tmp, sizeof(tmp))); >>> >>> That means if entries is >= 1000 for example, num_len is 4 ... >>> >>>> + /* Convert int to string */ >>>> + for (i = num_len - 1; i >= 0; i--) { >>>> + str[i] = num % 10 + '0'; >>>> + num /= 10; >>>> + } >>>> + >>>> + str[num_len] = '\0'; >>> ... and then you run into a buffer overflow here. >> >> Doh, you're correct. I forgot to put a "<=" in the len / num_len check. >> That should fix things up. Thanks for catching that. >> >> >>>> + return str; >>>> +} >>> Maybe it would also make more sense to panic() instead of "return 0" >>> since you don't check the return value in patch 5 ? >> >> I'm a bit conflicted about doing something like that. I'm not sure if >> there's any kind >> of guideline we want to follow for defining functions in libc. >> >> I see one of two possibilities: >> >> a. define these functions as "libc-like" as possible, and use them as >> if they were >> regular standard libc functions >> >> or >> >> b. change up these functions to better fit their use cases in >> pc-bios/s390-ccw >> >> Does that make sense? What do you think? > Keeping them libc-like likely makes sense ... but could we somehow also > make sure that we're not running into unexpected errors when using them? > Something like "IPL_assert(entries < 1000, ...)" before calling the > functions in patch 5? > > Thomas > > Sounds good to me.
On 12/19/2017 11:29 AM, Collin L. Walling wrote: > On 12/19/2017 02:31 AM, Thomas Huth wrote: >> On 18.12.2017 17:16, Collin L. Walling wrote: >>> On 12/18/2017 08:06 AM, Thomas Huth wrote: >>>> On 11.12.2017 23:19, Collin L. Walling wrote: >>>>> Moved: >>>>> memcmp from bootmap.h to libc.h (renamed from _memcmp) >>>>> strlen from sclp.c to libc.h (renamed from _strlen) >>>>> >>>>> Added C standard functions: >>>>> isdigit >>>>> atoi >>>>> >>>>> Added non-C standard function: >>>>> itostr >>>>> >>>>> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> >>>>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> >>>>> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> >>>>> --- >>>>> pc-bios/s390-ccw/Makefile | 2 +- >>>>> pc-bios/s390-ccw/bootmap.c | 4 +-- >>>>> pc-bios/s390-ccw/bootmap.h | 16 +--------- >>>>> pc-bios/s390-ccw/libc.c | 75 >>>>> ++++++++++++++++++++++++++++++++++++++++++++++ >>>>> pc-bios/s390-ccw/libc.h | 31 +++++++++++++++++++ >>>>> pc-bios/s390-ccw/main.c | 17 +---------- >>>>> pc-bios/s390-ccw/sclp.c | 10 +------ >>>>> 7 files changed, 112 insertions(+), 43 deletions(-) >>>>> create mode 100644 pc-bios/s390-ccw/libc.c >>>> [...] >>>>> + >>>>> +/** >>>>> + * itostr: >>>>> + * @num: the integer to be converted. >>>>> + * @str: a pointer to a string to store the conversion. >>>>> + * @len: the length of the passed string. >>>>> + * >>>>> + * Given an integer @num, convert it to a string. The string @str >>>>> must be >>>>> + * allocated beforehand. The resulting string will be null >>>>> terminated and >>>>> + * returned. >>>>> + * >>>>> + * Returns: the string @str of the converted integer @num. >>>>> + */ >>>>> +char *itostr(int num, char *str, size_t len) >>>>> +{ >>>>> + long num_len = 1; >>>>> + int tmp = num; >>>>> + int i; >>>>> + >>>>> + /* Count length of num */ >>>>> + while ((tmp /= 10) > 0) { >>>>> + num_len++; >>>>> + } >>>>> + >>>>> + /* Check if we have enough space for num and null */ >>>>> + if (len < num_len) { >>>>> + return 0; >>>>> + } >>>> I'm afraid, but I think you've got an off-by-one bug in this code. >>>> >>>> In patch 5, you're using this function like this: >>>> >>>> char tmp[4]; >>>> >>>> sclp_print(itostr(entries, tmp, sizeof(tmp))); >>>> >>>> That means if entries is >= 1000 for example, num_len is 4 ... >>>> >>>>> + /* Convert int to string */ >>>>> + for (i = num_len - 1; i >= 0; i--) { >>>>> + str[i] = num % 10 + '0'; >>>>> + num /= 10; >>>>> + } >>>>> + >>>>> + str[num_len] = '\0'; >>>> ... and then you run into a buffer overflow here. >>> >>> Doh, you're correct. I forgot to put a "<=" in the len / num_len >>> check. >>> That should fix things up. Thanks for catching that. >>> >>> >>>>> + return str; >>>>> +} >>>> Maybe it would also make more sense to panic() instead of "return 0" >>>> since you don't check the return value in patch 5 ? >>> >>> I'm a bit conflicted about doing something like that. I'm not sure if >>> there's any kind >>> of guideline we want to follow for defining functions in libc. >>> >>> I see one of two possibilities: >>> >>> a. define these functions as "libc-like" as possible, and use them as >>> if they were >>> regular standard libc functions >>> >>> or >>> >>> b. change up these functions to better fit their use cases in >>> pc-bios/s390-ccw >>> >>> Does that make sense? What do you think? >> Keeping them libc-like likely makes sense ... but could we somehow also >> make sure that we're not running into unexpected errors when using them? >> Something like "IPL_assert(entries < 1000, ...)" before calling the >> functions in patch 5? >> >> Thomas >> >> > > Sounds good to me. > What if we made a wrapper function for itostr. This func will have a tmp variable that stores the return of itostr. We then do an assertion to make sure we did not return 0 (which indicates that the size of the array was not large enough). If we pass, then just return tmp. e.g. static char *_itostr(int num, char *str, size_t len) { ... } char *itostr(int num, char *str, size_t len) { char *tmp = _itostr(num, str, len); IPL_assert(tmp != 0, "array too small for itostr conversion"); return tmp; } And as a side note: we know that the number of boot table entries for both ECKD DASD andSCSI cannot exceed 31, so we should be safe with a rather small value for our char arrays.
On 19.12.2017 21:23, Collin L. Walling wrote: > On 12/19/2017 11:29 AM, Collin L. Walling wrote: >> On 12/19/2017 02:31 AM, Thomas Huth wrote: >>> On 18.12.2017 17:16, Collin L. Walling wrote: >>>> On 12/18/2017 08:06 AM, Thomas Huth wrote: >>>>> On 11.12.2017 23:19, Collin L. Walling wrote: >>>>>> Moved: >>>>>> memcmp from bootmap.h to libc.h (renamed from _memcmp) >>>>>> strlen from sclp.c to libc.h (renamed from _strlen) >>>>>> >>>>>> Added C standard functions: >>>>>> isdigit >>>>>> atoi >>>>>> >>>>>> Added non-C standard function: >>>>>> itostr >>>>>> >>>>>> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> >>>>>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> >>>>>> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com> >>>>>> --- >>>>>> pc-bios/s390-ccw/Makefile | 2 +- >>>>>> pc-bios/s390-ccw/bootmap.c | 4 +-- >>>>>> pc-bios/s390-ccw/bootmap.h | 16 +--------- >>>>>> pc-bios/s390-ccw/libc.c | 75 >>>>>> ++++++++++++++++++++++++++++++++++++++++++++++ >>>>>> pc-bios/s390-ccw/libc.h | 31 +++++++++++++++++++ >>>>>> pc-bios/s390-ccw/main.c | 17 +---------- >>>>>> pc-bios/s390-ccw/sclp.c | 10 +------ >>>>>> 7 files changed, 112 insertions(+), 43 deletions(-) >>>>>> create mode 100644 pc-bios/s390-ccw/libc.c >>>>> [...] >>>>>> + >>>>>> +/** >>>>>> + * itostr: >>>>>> + * @num: the integer to be converted. >>>>>> + * @str: a pointer to a string to store the conversion. >>>>>> + * @len: the length of the passed string. >>>>>> + * >>>>>> + * Given an integer @num, convert it to a string. The string @str >>>>>> must be >>>>>> + * allocated beforehand. The resulting string will be null >>>>>> terminated and >>>>>> + * returned. >>>>>> + * >>>>>> + * Returns: the string @str of the converted integer @num. >>>>>> + */ >>>>>> +char *itostr(int num, char *str, size_t len) >>>>>> +{ >>>>>> + long num_len = 1; >>>>>> + int tmp = num; >>>>>> + int i; >>>>>> + >>>>>> + /* Count length of num */ >>>>>> + while ((tmp /= 10) > 0) { >>>>>> + num_len++; >>>>>> + } >>>>>> + >>>>>> + /* Check if we have enough space for num and null */ >>>>>> + if (len < num_len) { >>>>>> + return 0; >>>>>> + } >>>>> I'm afraid, but I think you've got an off-by-one bug in this code. >>>>> >>>>> In patch 5, you're using this function like this: >>>>> >>>>> char tmp[4]; >>>>> >>>>> sclp_print(itostr(entries, tmp, sizeof(tmp))); >>>>> >>>>> That means if entries is >= 1000 for example, num_len is 4 ... >>>>> >>>>>> + /* Convert int to string */ >>>>>> + for (i = num_len - 1; i >= 0; i--) { >>>>>> + str[i] = num % 10 + '0'; >>>>>> + num /= 10; >>>>>> + } >>>>>> + >>>>>> + str[num_len] = '\0'; >>>>> ... and then you run into a buffer overflow here. >>>> >>>> Doh, you're correct. I forgot to put a "<=" in the len / num_len >>>> check. >>>> That should fix things up. Thanks for catching that. >>>> >>>> >>>>>> + return str; >>>>>> +} >>>>> Maybe it would also make more sense to panic() instead of "return 0" >>>>> since you don't check the return value in patch 5 ? >>>> >>>> I'm a bit conflicted about doing something like that. I'm not sure if >>>> there's any kind >>>> of guideline we want to follow for defining functions in libc. >>>> >>>> I see one of two possibilities: >>>> >>>> a. define these functions as "libc-like" as possible, and use them as >>>> if they were >>>> regular standard libc functions >>>> >>>> or >>>> >>>> b. change up these functions to better fit their use cases in >>>> pc-bios/s390-ccw >>>> >>>> Does that make sense? What do you think? >>> Keeping them libc-like likely makes sense ... but could we somehow also >>> make sure that we're not running into unexpected errors when using them? >>> Something like "IPL_assert(entries < 1000, ...)" before calling the >>> functions in patch 5? >>> >>> Thomas >>> >>> >> >> Sounds good to me. >> > > > What if we made a wrapper function for itostr. This func will have a tmp > variable > that stores the return of itostr. We then do an assertion to make sure > we did not > return 0 (which indicates that the size of the array was not large > enough). If we > pass, then just return tmp. > > e.g. > > static char *_itostr(int num, char *str, size_t len) > { > ... > } > > char *itostr(int num, char *str, size_t len) > { > char *tmp = _itostr(num, str, len); > IPL_assert(tmp != 0, "array too small for itostr conversion"); > return tmp; > } That's fine for me, too. Thomas
diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile index 6d0c2ee..9f7904f 100644 --- a/pc-bios/s390-ccw/Makefile +++ b/pc-bios/s390-ccw/Makefile @@ -9,7 +9,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw) .PHONY : all clean build-all -OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o virtio-blkdev.o +OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o virtio-blkdev.o libc.o QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS)) QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c index 67a6123..6f8e30f 100644 --- a/pc-bios/s390-ccw/bootmap.c +++ b/pc-bios/s390-ccw/bootmap.c @@ -512,7 +512,7 @@ static bool is_iso_bc_entry_compatible(IsoBcSection *s) "Failed to read image sector 0"); /* Checking bytes 8 - 32 for S390 Linux magic */ - return !_memcmp(magic_sec + 8, linux_s390_magic, 24); + return !memcmp(magic_sec + 8, linux_s390_magic, 24); } /* Location of the current sector of the directory */ @@ -641,7 +641,7 @@ static uint32_t find_iso_bc(void) if (vd->type == VOL_DESC_TYPE_BOOT) { IsoVdElTorito *et = &vd->vd.boot; - if (!_memcmp(&et->el_torito[0], el_torito_magic, 32)) { + if (!memcmp(&et->el_torito[0], el_torito_magic, 32)) { return bswap32(et->bc_offset); } } diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h index cf99a4c..4980838 100644 --- a/pc-bios/s390-ccw/bootmap.h +++ b/pc-bios/s390-ccw/bootmap.h @@ -310,20 +310,6 @@ static inline bool magic_match(const void *data, const void *magic) return *((uint32_t *)data) == *((uint32_t *)magic); } -static inline int _memcmp(const void *s1, const void *s2, size_t n) -{ - int i; - const uint8_t *p1 = s1, *p2 = s2; - - for (i = 0; i < n; i++) { - if (p1[i] != p2[i]) { - return p1[i] > p2[i] ? 1 : -1; - } - } - - return 0; -} - static inline uint32_t iso_733_to_u32(uint64_t x) { return (uint32_t)x; @@ -416,7 +402,7 @@ const uint8_t vol_desc_magic[] = "CD001"; static inline bool is_iso_vd_valid(IsoVolDesc *vd) { - return !_memcmp(&vd->ident[0], vol_desc_magic, 5) && + return !memcmp(&vd->ident[0], vol_desc_magic, 5) && vd->version == 0x1 && vd->type <= VOL_DESC_TYPE_PARTITION; } diff --git a/pc-bios/s390-ccw/libc.c b/pc-bios/s390-ccw/libc.c new file mode 100644 index 0000000..60c4b28 --- /dev/null +++ b/pc-bios/s390-ccw/libc.c @@ -0,0 +1,75 @@ +/* + * libc-style definitions and functions + * + * This code 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. + */ + +#include "libc.h" + +/** + * atoi: + * @str: the string to be converted. + * + * Given a string @str, convert it to an integer. Any non-numerical value + * will terminate the conversion. + * + * Returns: an integer converted from the string @str. + */ +int atoi(const char *str) +{ + int i; + int val = 0; + + for (i = 0; str[i]; i++) { + char c = str[i]; + if (!isdigit(c)) { + break; + } + val *= 10; + val += c - '0'; + } + + return val; +} + +/** + * itostr: + * @num: the integer to be converted. + * @str: a pointer to a string to store the conversion. + * @len: the length of the passed string. + * + * Given an integer @num, convert it to a string. The string @str must be + * allocated beforehand. The resulting string will be null terminated and + * returned. + * + * Returns: the string @str of the converted integer @num. + */ +char *itostr(int num, char *str, size_t len) +{ + long num_len = 1; + int tmp = num; + int i; + + /* Count length of num */ + while ((tmp /= 10) > 0) { + num_len++; + } + + /* Check if we have enough space for num and null */ + if (len < num_len) { + return 0; + } + + /* Convert int to string */ + for (i = num_len - 1; i >= 0; i--) { + str[i] = num % 10 + '0'; + num /= 10; + } + + str[num_len] = '\0'; + + return str; +} diff --git a/pc-bios/s390-ccw/libc.h b/pc-bios/s390-ccw/libc.h index 0142ea8..00268e3 100644 --- a/pc-bios/s390-ccw/libc.h +++ b/pc-bios/s390-ccw/libc.h @@ -42,4 +42,35 @@ static inline void *memcpy(void *s1, const void *s2, size_t n) return s1; } +static inline int memcmp(const void *s1, const void *s2, size_t n) +{ + int i; + const uint8_t *p1 = s1, *p2 = s2; + + for (i = 0; i < n; i++) { + if (p1[i] != p2[i]) { + return p1[i] > p2[i] ? 1 : -1; + } + } + + return 0; +} + +static inline size_t strlen(const char *str) +{ + size_t i; + for (i = 0; *str; i++) { + str++; + } + return i; +} + +static inline int isdigit(int c) +{ + return (c >= '0') && (c <= '9'); +} + +int atoi(const char *str); +char *itostr(int num, char *str, size_t len); + #endif diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c index 401e9db..a8ef120 100644 --- a/pc-bios/s390-ccw/main.c +++ b/pc-bios/s390-ccw/main.c @@ -40,22 +40,7 @@ void panic(const char *string) unsigned int get_loadparm_index(void) { - const char *lp = loadparm; - int i; - unsigned int idx = 0; - - for (i = 0; i < 8; i++) { - char c = lp[i]; - - if (c < '0' || c > '9') { - break; - } - - idx *= 10; - idx += c - '0'; - } - - return idx; + return atoi(loadparm); } static bool find_dev(Schib *schib, int dev_no) diff --git a/pc-bios/s390-ccw/sclp.c b/pc-bios/s390-ccw/sclp.c index b1fc8ff..486fce1 100644 --- a/pc-bios/s390-ccw/sclp.c +++ b/pc-bios/s390-ccw/sclp.c @@ -65,14 +65,6 @@ void sclp_setup(void) sclp_set_write_mask(); } -static int _strlen(const char *str) -{ - int i; - for (i = 0; *str; i++) - str++; - return i; -} - long write(int fd, const void *str, size_t len) { WriteEventData *sccb = (void *)_sccb; @@ -95,7 +87,7 @@ long write(int fd, const void *str, size_t len) void sclp_print(const char *str) { - write(1, str, _strlen(str)); + write(1, str, strlen(str)); } void sclp_get_loadparm_ascii(char *loadparm)