Skip to content
This repository was archived by the owner on Nov 9, 2022. It is now read-only.

Fix oacr build warnings #68

Open
kasontey wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Fix oacr build warnings #68

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
78c79e8
resolving oacr build warnings
Jun 11, 2018
db652ee
taking away unintentional changes
Jun 11, 2018
8a6c337
CR feedback
Jun 19, 2018
7d6bc77
moving back to sizeof and adding TRUNCATE
Jun 19, 2018
b31e730
fixing linux build error
Jun 22, 2018
c028bd2
adding stdlib for _TRUNCATE use
Jun 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions libftl/ftl_private.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@
#define strcpy_s(dst, dstsz, src) strcpy(dst, src)
#define _strdup(src) strdup(src)
#define sscanf_s sscanf
#define memcpy_s(dst, dstsz, src, cnt) memcpy(dst, src, cnt)
#define vsnprintf_s(buf, bufsz, cnt, fmt, __VA_ARGS__) vsnprintf(buf, cnt, fmt, __VA_ARGS__)
#endif

typedef enum {
Expand Down
6 changes: 5 additions & 1 deletion libftl/handshake.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,11 @@ ftl_status_t _init_control_connection(ftl_stream_configuration_private_t *ftl) {
return retval;
}

// Suppressing getaddrinfo warning here, Windows prefers GetAddrInfoW,etc. but this doesn't exist on Linux
#pragma warning(push)
#pragma warning(disable:38026)
err = getaddrinfo(ftl->ingest_hostname, ingest_port_str, &hints, &resolved_names);
#pragma warning(pop)
if (err != 0) {
FTL_LOG(ftl, FTL_LOG_ERROR, "getaddrinfo failed to look up ingest address %s.", ftl->ingest_hostname);
FTL_LOG(ftl, FTL_LOG_ERROR, "gai error was: %s", gai_strerror(err));
Expand Down Expand Up @@ -366,7 +370,7 @@ static ftl_response_code_t _ftl_send_command(ftl_stream_configuration_private_t

memset(buf, 0, buflen);

len = vsnprintf(buf, buflen, format, valist);
len = vsnprintf_s(buf, buflen, MAX_INGEST_COMMAND_LEN, format, valist);

va_end(valist);

Expand Down
5 changes: 5 additions & 0 deletions libftl/ingest.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,12 @@ static int _ping_server(const char *hostname, int port) {

snprintf(port_str, 10, "%d", port);

// Suppressing getaddrinfo warning here, Windows prefers GetAddrInfoW,etc. but this doesn't exist on Linux
#pragma warning(push)
#pragma warning(disable:38026)
err = getaddrinfo(hostname, port_str, &hints, &resolved_names);
#pragma warning(pop)

if (err != 0) {
return FTL_DNS_FAILURE;
}
Expand Down
2 changes: 1 addition & 1 deletion libftl/logging.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ void ftl_log_msg(ftl_stream_configuration_private_t *ftl, ftl_log_severity_t log

m.msg.log.log_level = log_level;
va_start(args, fmt);
vsnprintf(m.msg.log.string, sizeof(m.msg.log.string), fmt, args);
vsnprintf_s(m.msg.log.string, sizeof(m.msg.log.string), sizeof(m.msg.log.string), fmt, args);
Copy link
Copy Markdown
Contributor

@QuinnDamerell-MS QuinnDamerell-MS Jun 12, 2018

Choose a reason for hiding this comment

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

I believe you want something like strlen here, sizeof will return the size of the type which I think is char*, so 4 bytes.

Copy link
Copy Markdown
Author

@kasontey kasontey Jun 19, 2018

Choose a reason for hiding this comment

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

Can you call strlen on a char[] ?

Copy link
Copy Markdown

@Doridian Doridian Jun 19, 2018

Choose a reason for hiding this comment

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

Of course, [] and * is the same in C anyway. (Both are a pointer, it is just syntactic sugar, really).
Also sizeof(char*) is 8 bytes on 64-bit :p

Copy link
Copy Markdown
Author

@kasontey kasontey Jun 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

hmmm, but there should be a nice way to get the count

Copy link
Copy Markdown
Contributor

@QuinnDamerell-MS QuinnDamerell-MS Jun 19, 2018

Choose a reason for hiding this comment

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

Yeah, strlen will look through the array until it finds a '/0'

Copy link
Copy Markdown

@TomyLobo TomyLobo Jun 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I was confused about your page numbers there for a while.
Apparently, pdf page 92 is standard page 80 :)

Anyway, I actually have no idea what type the "string" field has. I assumed that, being a field it was not char[], but char[x] (which behave differently in sizeof, at least in C++)

So, what is it? char[] or char[x]?

Copy link
Copy Markdown
Contributor

@dgarrett-microsoft dgarrett-microsoft Jun 19, 2018

Choose a reason for hiding this comment

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

Ah, sorry about the page confusion.
The "string" variable is a char[1024] in a struct which is in the msg union, which is in the m struct.

typedef struct {
  int log_level;
  char string[1024];
}ftl_status_log_msg_t;

Copy link
Copy Markdown

@TomyLobo TomyLobo Jun 19, 2018

Choose a reason for hiding this comment

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

https://ideone.com/1YQUTV there we go
So this means that "m.msg.log.string" cannot actually be a char[], assuming this piece of code compiles in gcc 6.3 :)

This means it's likely char[x] and sizeof was correct.
If, however, it is char*, you cannot use sizeof. In that case, you need the size from somewhere else. If it's not initialized and you did not store the length somewhere alongside the pointer, good luck figuring out the length :)

Copy link
Copy Markdown
Author

@kasontey kasontey Jun 19, 2018

Choose a reason for hiding this comment

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

Yup so seems like this is a char[x], so I'll move this back to sizeof.
And for the count we'll use the _TRUNCATE for string truncation.

Copy link
Copy Markdown
Contributor

@QuinnDamerell-MS QuinnDamerell-MS Jun 22, 2018

Choose a reason for hiding this comment

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

Good call. Thanks everyone!

va_end(args);

enqueue_status_msg(ftl, &m);
Expand Down
2 changes: 1 addition & 1 deletion libftl/media.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,7 @@ static int _media_send_slot(ftl_stream_configuration_private_t *ftl, nack_slot_t
int pkt_len;

os_lock_mutex(&ftl->media.mutex);
memcpy(pkt, slot->packet, slot->len);
memcpy_s(pkt, sizeof(pkt), slot->packet, slot->len);
Copy link
Copy Markdown
Contributor

@QuinnDamerell-MS QuinnDamerell-MS Jun 12, 2018

Choose a reason for hiding this comment

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

This should be sizeof(pkt) * MAX_PACKET_BUFFER

pkt_len = slot->len;
os_unlock_mutex(&ftl->media.mutex);

Expand Down