C: Am I Passing the String Wrong?

Question

I'm trying to make a utility for my stream. Basically, OBS can read from a text file, so I'm coding a program that updates a text file with the current time.

The program would accept a string to append in front of the actual time. It's defined in main(), and passed to writeTime(), which executes every second and does the writing.

However, something's going wrong, and I can't show the prefix even if I define it. I think there's something wrong with the way that I pass the argument to writeTime(), but I can't figure out what the issue is.

My code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <unistd.h>

// This program writes the current time into a text file every second.
int writeTime(short int timezone, char prefix[], short int h24){
    // Open the text file, in which to write
    FILE *filePointer;
    filePointer = fopen("clock.txt", "w+");
    if (filePointer == NULL) {
        printf("The file clock.txt failed to open.");
    } else {
        // THIS PART DOESN'T QUITE WORK WITH THE PREFIX:
        time_t now = time(NULL); // get system time in seconds from 1970-01-01
        int timeOfDay = (now + timezone * 3600) % 86400;
        short int hour = timeOfDay / 3600;
        short int minute = timeOfDay % 3600 / 60;
        short int second = timeOfDay % 60;
        if (h24) { // h24 is the 24 hour time flag
            printf("%s %02i:%02i.%02i\n", prefix, hour, minute, second);
            fprintf(filePointer, "%s %02i:%02i.%02i", prefix, hour, minute, second);
        } else {
            char* ampm;
            if (hour < 12) {
                ampm = "AM";
            } else {
                ampm = "PM";
            }
            printf("%s %02i:%02i.%02i %s\n", prefix, hour%12, minute, second, ampm);
            fprintf(filePointer, "%s %02i:%02i.%02i %s", prefix, hour%12, minute, second, ampm);
        }

    }
    fclose(filePointer);
    return 0;
}

int main(int argc, char **argv){
    // Flags : 00000HPZ, 1 if set from command linearguments
    // Z: timezone, P: prefix, H: 24h mode
    unsigned short int flags = 0;
    short int timezone = 0;
    char prefix[64] = "";
    short int h24 = 0;
    // This part is meant to get parameters from command line arguments to save time for reuse
    // -z: timezone; -p: prefix string; -24: 24h time (1 or 0 = on or off)
    for (int i = 1; i < argc; ++i) {
        if (0 == strcmp(argv[i], "-z")) {
            timezone = (int) strtol(argv[++i], (char **)NULL, 10);
            flags |= 1;
            printf("Timezone UTC%+i\n", timezone);
        } else if (0 == strcmp(argv[i], "-p")) {
            strcpy(prefix, argv[++i]);
            flags |= 2;
            printf("Prefix %s\n",prefix);
        } else if (0 == strcmp(argv[i], "-24")) {
            h24 = (int) strtol(argv[++i], (char **)NULL, 10);
            flags |= 4;
            printf("24h %i\n", h24);
        }
    }
    // User input for parameters not gotten from arguments:
    if (!(flags & 1)) {
        printf("Enter your timezone, as offset from UTC (e.g. East Coast US = -5, or -4 during DST): ");
        scanf("%i", &timezone);
        printf("UTC%+i\n", timezone);
    }
    if (!(flags & 1 << 1)) {
        printf("Enter your prefix (e.g. \"Local time:\", up to 64 characters) ");
        // flush the input buffer: https://stackoverflow.com/questions/7898215/how-to-clear-input-buffer-in-c
        int c;
        while ((c = getchar()) != '\n' && c != EOF) {}
        gets(prefix);
        puts(prefix);
    }
    if (!(flags & 1 << 2)) {
        printf("Enter \"1\" to enable 24 hour mode, and \"0\" to enable 12 hour mode. ");
        scanf("%i", &h24);
        printf("24h %i\n", h24);
    }
    // Main loop
    while (1) {
        // AM I DOING THIS PART RIGHT???
        writeTime(timezone, prefix, h24);
        sleep(1);
    }
    return 0;
}

why don't you use the system functions, `asctime`, `strftime` or `ctime`? — bruceg, Mar 20 '19 at 23:01
I'm on Windows, and it Works On My Machine (tm). I fixed the scanf format strings, there are two places where you write into a short but use %i instead of %h. I seriously doubt that's the issue you're having, but it's what I changed and I'm getting a steady once a second printing of time with "Local time:" in front of it. — brothir, Mar 20 '19 at 23:02
Running the posted code through the `gcc` compiler results in LOTS of warning and errors. For instance: "`untitled2.c:67:17: warning: format ‘%i’ expects argument of type ‘int *’, but argument 2 has type ‘short int *’ [-Wformat=]` for statement: `scanf("%i", &timezone);` Suggest correcting the input format specifier. Similar considerations exist for the statement: `scanf("%i", &h24);` — user3629249, Mar 20 '19 at 23:06
regarding: `gets(prefix);` the function: `gets()` has been depreciated for years and completely removed from the latest versions of the C language. Suggest using `fgets()` (read the MAN page as the parameter list is different) — user3629249, Mar 20 '19 at 23:08
When compiling, always enable the warnings, then fix those warnings. ( for `gcc`, at a minimum use: `-Wall -Wextra -Wconversion -pedantic -std=gnu11` ) Note: other compilers use different options to produce the same thing — user3629249, Mar 20 '19 at 23:10
Suggest using the input format specifier (for the short int variables) `%hi` — user3629249, Mar 20 '19 at 23:13
regarding this kind of statement: `fprintf(filePointer, "%s %02i:%02i.%02i", prefix, hour, minute, second);` The format string is missing a trailing '\n' so the data will stay in the stream buffer until one of several events happens. Suggest appending a '\n' to the end of the format string — user3629249, Mar 20 '19 at 23:24

score 2 · Answer 1 · answered Mar 20 '19 at 23:25

2

Yeah, that should be fine. But you're using scanf before you get there

short int h24;
...
scanf("%i", &h24);

%i assumes an int not a short. So it writes 4 bytes to a 2 byte variable. That overflows into your string. It clobbers prefix[0] to 0 on mine, so the string is 0-length.

Use scanf("%h", &h24); instead.

answered Mar 20 '19 at 23:25

Philip

1,539
14
23

So it really was that. – brothir Mar 20 '19 at 23:33

user3629249 · Accepted Answer · 2019-03-20T23:51:39.940

the following proposed code:

cleanly compiles
performs the desired functionality
look for <-- corrected to see what I modified

and now, the proposed code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <unistd.h>

// This program writes the current time into a text file every second.
int writeTime(short int timezone, char prefix[], short int h24)
{
    // Open the text file, in which to write
    FILE *filePointer;
    filePointer = fopen("clock.txt", "w+");

    if (filePointer == NULL) 
    {
        perror("The file clock.txt failed to open."); // <-- corrected
    } 

    else 
    {
        // THIS PART DOESN'T QUITE WORK WITH THE PREFIX:
        time_t now = time(NULL); // get system time in seconds from 1970-01-01
        time_t timeOfDay = (now + timezone * 3600) % 86400;
        short int hour = (short)(timeOfDay / 3600); // <-- corrected
        short int minute = (short)(timeOfDay % 3600 / 60); // <-- corrected
        short int second = (short)(timeOfDay % 60); // <-- corrected

        if (h24) { // h24 is the 24 hour time flag
            printf("%s %02i:%02i.%02i\n", prefix, hour, minute, second);
            fprintf(filePointer, "%s %02i:%02i.%02i\n", prefix, hour, minute, second);  // <-- corrected
        } 

        else 
        {
            char* ampm;

            if (hour < 12) 
            {
                ampm = "AM";
            } 

            else 
            {
                ampm = "PM";
            }

            printf("%s %02i:%02i.%02i %s\n", prefix, hour%12, minute, second, ampm);
            fprintf(filePointer, "%s %02i:%02i.%02i %s\n", prefix, hour%12, minute, second, ampm); // <-- corrected
        }
    }
    fclose(filePointer);
    return 0;
}


int main(int argc, char **argv){
    // Flags : 00000HPZ, 1 if set from command linearguments
    // Z: timezone, P: prefix, H: 24h mode
    unsigned short int flags = 0;
    short int timezone = 0;
    char prefix[64] = "";
    short int h24 = 0;

    // This part is meant to get parameters from command line arguments to save time for reuse
    // -z: timezone; -p: prefix string; -24: 24h time (1 or 0 = on or off)
    for (int i = 1; i < argc; ++i) 
    {
        if (0 == strcmp(argv[i], "-z")) 
        {
            timezone = ( short int) strtol(argv[++i], NULL, 10);  // <-- corrected
            flags |= 1;
            printf("Timezone UTC%+i\n", timezone);
        } 

        else if (0 == strcmp(argv[i], "-p")) 
        {
            strcpy(prefix, argv[++i]);
            flags |= 2;
            printf("Prefix %s\n",prefix);
        } 

        else if (0 == strcmp(argv[i], "-24")) 
        {
            h24 = (short int) strtol(argv[++i], NULL, 10);  // <-- corrected
            flags |= 4;
            printf("24h %i\n", h24);
        }
    }

    // User input for parameters not gotten from arguments:
    if (!(flags & 1)) 
    {
        printf("Enter your timezone, as offset from UTC (e.g. East Coast US = -5, or -4 during DST): ");
        scanf("%hi", &timezone);  // <-- corrected
        printf("UTC%+i\n", timezone);
    }

    if (!(flags & 1 << 1)) 
    {
        printf("Enter your prefix (e.g. \"Local time:\", up to 64 characters) ");
        // flush the input buffer: 
        int c;
        while ((c = getchar()) != '\n' && c != EOF) {}

        // replace the call to `gets()` with:
        fgets( prefix, sizeof(prefix), stdin );
        puts(prefix);
    }
    if (!(flags & 1 << 2)) {
        printf("Enter \"1\" to enable 24 hour mode, and \"0\" to enable 12 hour mode. ");
        scanf("%hi", &h24); // <-- corrected
        printf("24h %i\n", h24);
    }

    // Main loop
    while (1) 
    {
        // AM I DOING THIS PART RIGHT???
        writeTime(timezone, prefix, h24);
        sleep(1);
    }

    return 0;
}

The terminal output is:

Enter your timezone, as offset from UTC (e.g. East Coast US = -5, or -4 during DST): -7
UTC-7
Enter your prefix (e.g. "Local time:", up to 64 characters) pst
pst

Enter "1" to enable 24 hour mode, and "0" to enable 12 hour mode. 1
24h 1

pst
 16:34.58
pst
 16:34.59
pst
 16:35.00
pst
 16:35.01
pst
 16:35.02
pst
 16:35.03
pst
 16:35.04
pst
 16:35.05
pst
 16:35.06
....

the disk file: clock.txt contains:

pst
 16:32.48

and is updated once per second

This worked for the most part! I'll have to remove the `\n` at the end of `prefix` though... — haley, Mar 21 '19 at 02:45

C: Am I Passing the String Wrong?

2 Answers2