Alloc fail & fix compile warning #105

Hojun-Cho · 2023-07-12T03:42:27Z

I fix some compile warning and alloc fail(#104 ).
But still occur two compile warning. Do you want to fix this complie warning?

Here's my environment

GCC : 13.1.1
Arch: Linux arch 6.3.8-arch1-1 x86_64

extensions/qconfig.c: In function ‘qconfig_parse_file’:
extensions/qconfig.c:165:48: warning: ‘%s’ directive output may be truncated writing up to 4094 bytes into a region of size between 1 and 4095 [-Wformat-truncation=]
  165 |                 snprintf(tmp, sizeof(tmp), "%s/%s", dir, buf);
      |                                                ^~        ~~~
extensions/qconfig.c:165:17: note: ‘snprintf’ output between 2 and 8190 bytes into a destination of size 4096
  165 |                 snprintf(tmp, sizeof(tmp), "%s/%s", dir, buf);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

gcc -g -O2 -Wall -Wstrict-prototypes -fPIC -D_GNU_SOURCE -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include -I/usr/local/include -I../include/qlibc -I./internal -c -o utilities/qtime.o
 utilities/qtime.c                                                                                                                                                                             
In file included from /usr/include/bits/libc-header-start.h:33,
                 from /usr/include/stdio.h:27,
                 from utilities/qtime.c:36:
/usr/include/features.h:195:3: warning: #warning "_BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE" [-Wcpp]  
  195 | # warning "_BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE"

strncpy(newstr, str, strsize) => memcpy Because strsize is computed in 302 line, if call strncpy then recompute len.

wolkykim · 2023-07-12T04:51:45Z

src/utilities/qsocket.c

@@ -112,8 +112,9 @@ bool qsocket_close(int sockfd, int timeoutms) {
        char buf[1024];
        while (true) {
            ssize_t read = qio_read(sockfd, buf, sizeof(buf), timeoutms);
+			DEBUG("Throw %zu bytes from dummy input stream.", read);


seems the indentation is not lined up.

qlibc uses spaces, please ignore using tabs

Oops, didn't realize that, thanks for the heads up. Fixed

wolkykim · 2023-07-12T05:04:19Z

src/utilities/qstring.c

@@ -389,6 +395,9 @@ char *qstrdup_between(const char *str, const char *start, const char *end) {
    int len = e - s;

    char *buf = (char *) malloc(sizeof(char) * (len + 1));
+	if (buf == NULL)
+		return NULL;
+
    strncpy(buf, s, len);


maybe we can update this to memcpy() as well

SGTM. How about using qmemdup?

Your right. I'll change to memcpy.

wolkykim · 2023-07-12T05:07:10Z

Thanks for the PR. I left some minor comments.

Hojun-Cho · 2023-07-12T06:11:00Z

The comment you left is very helpful.

wolkykim · 2023-07-12T06:38:40Z

src/utilities/qstring.c

@@ -388,10 +393,8 @@ char *qstrdup_between(const char *str, const char *start, const char *end) {

    int len = e - s;

-    char *buf = (char *) malloc(sizeof(char) * (len + 1));
-    strncpy(buf, s, len);
+    char *buf = qmemdup(s, len);


Doesn't it need to be len + 1 and also need NULL check for the buf?
I'd like to focus on the NULL return on this PR. Let's not mix them in a single PR.
Can you revert this for now and just add the NULL check?

wolkykim · 2023-07-12T06:47:35Z

src/utilities/qio.c

@@ -303,7 +303,7 @@ ssize_t qio_puts(int fd, const char *str, int timeoutms) {
    char *newstr = (char *) malloc(strsize + 1 + 1);
    if (newstr == NULL)
        return -1;
-    strncpy(newstr, str, strsize);
+    memcpy(newstr, str, strsize);


Can you revert this line please? I'd prefer not to mix the change in this PR.

wolkykim · 2023-07-12T06:53:21Z

src/utilities/qstring.c


+    memcpy(buf, s, len);


also let's keep this to strncpy() for now.

memcpy => strncpy src/utilities/qio.c :306 src/utilities/qstring.c :400

Hojun-Cho · 2023-07-12T07:05:00Z

I applied the requests. Thanks for the good advice.

wolkykim · 2023-07-12T07:44:07Z

Thanks for working with me. A Great PR!!!

Hojun-Cho added 3 commits July 12, 2023 11:41

PR to wolkykim#104. Memory allocation fail then return NULL.

4b7cacc

fix compile warning utilities/qio.c qio_puts func

dde246d

strncpy(newstr, str, strsize) => memcpy Because strsize is computed in 302 line, if call strncpy then recompute len.

fix compile warning utilities/qsocket.c qsocket_close

4a652e7

wolkykim reviewed Jul 12, 2023

View reviewed changes

lined up utilities/qsocket.c

b7c7859

wolkykim reviewed Jul 12, 2023

View reviewed changes

tab to space

5a787dc

qmemdup => memcpy utilities/qstring.c

701e136

wolkykim reviewed Jul 12, 2023

View reviewed changes

wolkykim requested changes Jul 12, 2023

View reviewed changes

wolkykim reviewed Jul 12, 2023

View reviewed changes

src/utilities/qstring.c Outdated

memcpy(buf, s, len);

Copy link

Owner

wolkykim Jul 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also let's keep this to strncpy() for now.

revert memcpy -> strncpy

6a29214

memcpy => strncpy src/utilities/qio.c :306 src/utilities/qstring.c :400

Hojun-Cho requested a review from wolkykim July 12, 2023 07:05

wolkykim approved these changes Jul 12, 2023

View reviewed changes

wolkykim merged commit 1826858 into wolkykim:master Jul 12, 2023
1 check passed

Hojun-Cho deleted the alloc-fail branch July 12, 2023 08:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Alloc fail & fix compile warning #105

Alloc fail & fix compile warning #105

Hojun-Cho commented Jul 12, 2023

wolkykim Jul 12, 2023

wolkykim Jul 12, 2023

Hojun-Cho Jul 12, 2023 •

edited

Loading

wolkykim Jul 12, 2023

Hojun-Cho Jul 12, 2023

Hojun-Cho Jul 12, 2023

wolkykim commented Jul 12, 2023

Hojun-Cho commented Jul 12, 2023

wolkykim Jul 12, 2023 •

edited

Loading

Hojun-Cho Jul 12, 2023

wolkykim Jul 12, 2023

wolkykim Jul 12, 2023

Hojun-Cho commented Jul 12, 2023

wolkykim commented Jul 12, 2023

Alloc fail & fix compile warning #105

Alloc fail & fix compile warning #105

Conversation

Hojun-Cho commented Jul 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hojun-Cho Jul 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wolkykim commented Jul 12, 2023

Hojun-Cho commented Jul 12, 2023

wolkykim Jul 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hojun-Cho commented Jul 12, 2023

wolkykim commented Jul 12, 2023

Hojun-Cho Jul 12, 2023 •

edited

Loading

wolkykim Jul 12, 2023 •

edited

Loading