syslog formatted logger

Question

I have a logging example that I wrote myself and it currently consists of a single file.
In this code, I aim to reduce the number of parameters of the functions, increase the processing speed and make the code run operating system independent.
I also appreciated if you guys have any suggestions about fileing this eith an arch. I don't know any c arcs so. I am open to any suggestions and changes that will help.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <winsock2.h>
#include <stdint.h>

#ifdef WIN32
    #include <windows.h>
#else
    #include <unistd.h>
    #include <sys/types.h>
    #include <unistd.h>
#endif

#pragma comment(lib, "ws2_32.lib")

typedef enum {
    EMERGENCY = 0,
    ALERT,
    CRITICAL,
    SYS_ERROR,
    WARNING,
    NOTICE,
    INFO,
    DEBUG
} SyslogSeverity;

typedef enum {
    KERNEL_MESSAGES = 0,
    USER_LEVEL_MESSAGES,
    MAIL_SYSTEM,
    CLOCK_DAEMON
} SyslogFacility;

static uint32_t sequenceId = 0;

uint32_t calculatePriority(SyslogSeverity severity, SyslogFacility facility) {
    return (uint32_t)(facility * 8 + severity);
}

int32_t getTimestamp(char *buffer, uint32_t bufferSize) {
    time_t now = time(NULL);
    struct tm *tm_info = gmtime(&now);

    if (tm_info != NULL) {
        strftime(buffer, bufferSize, "%Y-%m-%dT%H:%M:%S.000Z", tm_info);
        return 0; // Success
    } else {
        strncpy(buffer, "1970-01-01T00:00:00.000Z", bufferSize);
        return -1; // Failure
    }
}

int32_t getSystemUptime(char *buffer, uint32_t bufferSize) {
#ifdef WIN32
    uint64_t uptime = GetTickCount64();
    snprintf(buffer, bufferSize, "%llu", uptime);
#else
    static clock_t start;
    static int initialized = 0;
    if (!initialized) {
        start = clock();
        initialized = 1;
    }
    clock_t now = clock();
    int64_t uptime = (now - start) * 100 / CLOCKS_PER_SEC;
    snprintf(buffer, bufferSize, "%lld", uptime);
#endif
    return 0; // Success
}

uint32_t getProcessId() {
#ifdef WIN32
    return (uint32_t)GetCurrentProcessId();
#else
    return (uint32_t)getpid();
#endif
}

int32_t formatLogMessage(char *logBuffer, uint32_t bufferSize, const char *message, const char *hostname, const char *messageId) {
    char timestamp[64];
    char sysUpTime[64];
    char structuredData[512];
    uint32_t procId = getProcessId();

    if (getTimestamp(timestamp, sizeof(timestamp)) != 0) {
        return -1; // Failure
    }

    if (getSystemUptime(sysUpTime, sizeof(sysUpTime)) != 0) {
        return -1; // Failure
    }

    snprintf(structuredData, sizeof(structuredData),
             "[timeQuality tzKnown=\"1\" isSynced=\"1\" syncAccuracy=\"60000\"] "
             "[origin software=\"MyApp\"] "
             "[meta sequenceId=\"%u\" sysUpTime=\"%s\" language=\"en\"]",
             ++sequenceId, sysUpTime);

    snprintf(logBuffer, bufferSize,
             "<%u>1 %s %s MyApplication %u %s %s %s",
             calculatePriority(INFO, CLOCK_DAEMON),
             timestamp, hostname, procId, messageId, structuredData, message);

    return 0; // Success
}

SOCKET createUdpSocket(const char *ipAddress, uint16_t port, struct sockaddr_in *serverAddr) {
    WSADATA wsaData;
    SOCKET sock;

    if (WSAStartup(MAKEWORD(2, 2), &wsaData) != 0) {
        printf("WSAStartup failed\n");
        return INVALID_SOCKET;
    }

    sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
    if (sock == INVALID_SOCKET) {
        printf("Socket creation failed\n");
        WSACleanup();
        return INVALID_SOCKET;
    }

    memset(serverAddr, 0, sizeof(*serverAddr));
    serverAddr->sin_family = AF_INET;
    serverAddr->sin_port = htons(port);
    serverAddr->sin_addr.s_addr = inet_addr(ipAddress);

    return sock;
}

int32_t sendUdpLog(SOCKET sock, const struct sockaddr_in *serverAddr, const char *logMessage) {
    int32_t sendResult = sendto(sock, logMessage, (int32_t)strlen(logMessage), 0, (struct sockaddr *)serverAddr, sizeof(*serverAddr));

    if (sendResult == SOCKET_ERROR) {
        printf("Sendto failed with error: %d\n", WSAGetLastError());
        return -1; // Failure
    } else {
        printf("Log sent: %s\n", logMessage);
        return 0; // Success
    }
}

int main(int32_t argc, char *argv[]) {
    if (argc != 3) {
        printf("Usage: %s <sleeptime> <hostname>\n", argv[0]);
        return 1;
    }

    int32_t sleepTime = atoi(argv[1]);
    const char *hostname = argv[2];
    const char *messageId = "ID12345";  // Example MSGID

    struct sockaddr_in serverAddr;
    SOCKET sock = createUdpSocket("127.0.0.1", 40514, &serverAddr);
    if (sock == INVALID_SOCKET) {
        return 1;
    }

    char logBuffer[1024];

    const int32_t numMessages = 3;
    for (int32_t i = 0; i < numMessages; ++i) {
        if (formatLogMessage(logBuffer, sizeof(logBuffer), "This is an informational message.", hostname, messageId) == 0) {
            if (sendUdpLog(sock, &serverAddr, logBuffer) != 0) {
                break;
            }
        }
        Sleep(sleepTime);
    }

    closesocket(sock);
    WSACleanup();

    return 0;
}

Answer 1

syslog spec

If there is some external specification the OP code strives to comply with, then please update this question to cite the spec.

redundant include

    #include <unistd.h>
    #include <sys/types.h>
    #include <unistd.h>

We can say it twice twice, but that won't make it any more uni-standard.

SyslogFacility

I am reading rfc 5424.

typedef enum {
    KERNEL_MESSAGES = 0,
    USER_LEVEL_MESSAGES,
    MAIL_SYSTEM,
    CLOCK_DAEMON [ = 3 ]
} SyslogFacility;

              3             system daemons
            ...
              9             clock daemon
            ...
             12             NTP subsystem

Something that receives Network Time Protocol (ntpd) sounds like a CLOCK_DAEMON to me. And then there's the good ol' clockd. Consider incorporating some linux header files into the windows port.

uptime

The semantics of getSystemUptime() is just amazing, something I never would have predicted from the name. And there's no helpful comment to explain the details.

Reporting uptime via a decimal ASCII string works, but is weird. Using the integer return value to report a failure that can never happen is a design decision worth revisiting. "Time since initial call" is pretty clearly different from "time since host rebooted". The syslog outputs don't mention whether we're on linux or windows, which makes it harder for reporting software to accurately interpret the meaning of "uptime" after logs from diverse hosts have been gathered in a central storage location.

nonstandard syntax

Maybe this is an ESL issue? Maybe this stems from speech-to-text transcription on a phone? I find that when I write outside my native tongue the text benefits greatly from a pass through Google Translate.

any suggestions about fileing this eith an arch. i dont know any c arcs so.

If there is an issue I can help with in machine architecture, or in software system architecture design, it's not yet clear to me what that might be.

short sleep

    int32_t sleepTime = atoi(argv[1]);

Consider supporting brief intervals, e.g. a 0.5 second sleep between messages.

Answer 2

Just some time feedback

• Consider leading with the buffer size. This helps various code analyzer tools.

• Also use size_t for array sizing and indexing.

• Since the return is either success/failure return a bool (or if stuck in the past, an int).

int32_t getTimestamp(char *buffer, uint32_t bufferSize) {
// versus
int32_t getTimestamp(uint32_t bufferSize, char *buffer) {
// even better
int32_t getTimestamp(uint32_t bufferSize, char buffer[bufferSize]) {
// even more better
int32_t getTimestamp(size_t bufferSize, char buffer[bufferSize]) {
// best so far
bool getTimestamp(size_t bufferSize, char buffer[bufferSize]) {

• strncpy() is a poor choice as the return with a small bufferSize is a character lacking a null character - not a string.

• Conceptually tm_info == NULL is clearer than tm_info != NULL. Or just go with if (tm_info). Useful to reduce negations.

• No need for an else when the prior if block has a return. Clearer to show that a missing return is not the case.

• Move toward a more like ISO 8601 timestamp.

// "%Y-%m-%dT%H:%M:%S.000Z"
"%04Y-%02m-%02dT%02H:%02M:%02S.000Z"

---

// Return success flag.
bool getTimestamp(size_t bufferSize, char buffer[bufferSize]) {
  time_t now = time(NULL);
  struct tm *tm_info = gmtime(&now);

  if (tm_info) {
    strftime(buffer, bufferSize, "%04Y-%02m-%02dT%02H:%02M:%02S.000Z", tm_info);
    return true; // Success
  }
  snprintf(buffer, (int) bufferSize, "1970-01-01T00:00:00.000Z");
  return false; // Failure
}

Also, since time() can return -1 for an error, I'd consider Dec 31, 1969, 23:59:59...` a better alternative error than Jan 1, 1970....

---

In retrospec, I'd advocate for setting the buffer to "" on error.

// Return success flag.
bool getTimestamp(size_t bufferSize, char buffer[bufferSize]) {
  time_t now = time(NULL);
  if (now == -1) {
    buffer[0] = '\0';
    return false; // Failure
  }
    
  struct tm *tm_info = gmtime(&now);

  if (tm_info == 0) {
    buffer[0] = '\0';
    return false; // Failure
  }
  if (strftime(buffer, bufferSize, "%04Y-%02m-%02dT%02H:%02M:%02S.000Z"
, tm_info) == 0) {
    buffer[0] = '\0';
    return false; // Failure
  }
  return true; // Success
}