2

Given the following two statements to use fgets and strip the newline:

puts("Enter the name.");
fgets(temp.name, 40, stdin);
temp.name[strcspn(temp.name, "\n")] = 0;

Is the following macro sufficient to take the place of this?

#define FGETS(str, len)  fgets(str, len, stdin); str[strcspn(str, "\n")] = 0
FGETS(temp.name, 40);

Are there any deficiencies or ways it can be improved?

Rachid K.
  • 4,490
  • 3
  • 11
  • 30
carl.hiass
  • 1,526
  • 1
  • 6
  • 26

4 Answers4

3

File input/output is a back-hole of time. Little to gain using a macro over a clean function.

Perhaps a function to do the prompt too?

Code uses a leading size per C2x priciple.

char *getaline(int size, char *s, const char *prompt) {
  if (prompt) {
    fputs(prompt, stdout);
  }
  if (fgets(s, size, stdin)) {
    // Perhaps add detection of failing to read the whole line?

    s[strcspn(s, "\n")] = '\0';
    return s;
  }
  if (size > 0) {
    *s = '\0';
  }
  return NULL;
}
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • I think we're on the same page. Your function is similar to ones I wrote for this answer: https://stackoverflow.com/questions/65011769/check-if-all-values-entered-into-char-array-are-numerical/65013419#65013419 – Craig Estey Mar 11 '21 at 00:57
  • 1
    @chux -- thanks, I just think there's one typo on the line `if (size > 0) [` should be `{`. – carl.hiass Mar 11 '21 at 02:29
2

Some remarks:

  • When a macro is composed with several instructions, it is preferable to surround it with "do { ... } while(0)" to be able to use it in "if" statements without brackets, prevent some arithmetic operations on the macros...
  • The parameters of the macros must be surrounded by parenthesis as they may not be simple variables but some expressions;
  • fgets() returns NULL if EOF is entered or upon error;

Hence the following proposition:

#define FGETS(str, len) do {  if (fgets((str), (len), stdin) != NULL) (str)[strcspn((str), "\n")] = 0; else (str)[0] = '\0'; } while(0)

Then you can use it as:

if (expression)
  FGETS(str, len);
else
  do_something_else;
...
FGETS(buf + 34, LEN);

N.B.: This page provides lots of tips to make secured macros.

Rachid K.
  • 4,490
  • 3
  • 11
  • 30
1
#define READLINE(str, len) do { if(fgets((str), (len), stdin) != NULL) { (str)[strcspn((str), "\n")] = 0; } else { /*TODO: Handle error*/ } } while (0)

Notice, I specifically renamed it from FGETS because it has hard-coded stdin as the "file" part. In my mind it's misleading to call it FGETS. Also, added brackets to arguments (pretty much a must for macros), wrapped it in a do - while(0) (here's why) and a basic framework for handling errors.

Govind Parmar
  • 20,656
  • 7
  • 53
  • 85
1

Usually, I do the do { } while (0); trick.

But, if you want a macro that is a drop in replacement for fgets where you can test the return value transparently, how about:

#include <stdio.h>
#include <string.h>

#define FGETS(_buf,_len,_xf) \
    ({ \
        char *_cp = fgets(_buf,_len,_xf); \
        if (_cp != NULL) \
            _buf[strcspn(_buf,"\n")] = 0; \
        _cp; \
    })

#define FGETOF(_buf,_xf) \
    FGETS(_buf,sizeof(_buf),_xf)

int
main(void)
{
    char buf[100];

    while (1) {
        if (FGETOF(buf,stdin) == NULL)
            break;
        printf("buf: '%s'\n",buf);
    }

    return 0;
}

The multiline macro is fine, but can be cleaned up [with no loss in speed] using an inline function:

#include <stdio.h>
#include <string.h>

static inline char *
xfgets(char *buf,size_t siz,FILE *xf)
{
    char *cp;

    cp = fgets(buf,siz,xf);

    if (cp != NULL)
        buf[strcspn(buf,"\n")] = 0;

    return cp;
}

#define FGETS(_buf,_len,_xf) \
    xfgets(_buf,_len,_xf)

#define FGETOF(_buf,_xf) \
    FGETS(_buf,sizeof(_buf),_xf)

int
main(void)
{
    char buf[100];

    while (1) {
        if (FGETOF(buf,stdin) == NULL)
            break;
        printf("buf: '%s'\n",buf);
    }

    return 0;
}
Craig Estey
  • 30,627
  • 4
  • 24
  • 48
  • why do you do two macros for `FGETS`...`FGETOF` instead of just one? – carl.hiass Mar 11 '21 at 00:24
  • 1
    It's [only] a nicety. The direct replacement is the `FGETS` macro. (e.g.) In `main`, we could do: `if (FGETS(buf,sizeof(buf),stdin) == NULL)`. But, using `sizeof(buf)` is common, so under the DRY [don't repeat yourself] principle, I added the `FGETOF` to _force_ the common usage of `sizeof(buf)` for the length. YMMV – Craig Estey Mar 11 '21 at 00:30
  • Similarly, I use a `#define ALLOCME(_ptr) _ptr = malloc(sizeof(*_ptr))` to eliminate some repetitive boilerplate. Sometimes, I have this macro call `xmalloc` [which I define], that calls `malloc` but checks the return value and aborts on `NULL`, since 99.44% of the time you want to abort if `malloc` fails. – Craig Estey Mar 11 '21 at 00:36
  • `FGETOF(buf,stdin) == NULL` looks like it using a C extension to get a return value from the macro. I like the idea of returning something to help detect EOF - using standard code. Hmmm. – chux - Reinstate Monica Mar 11 '21 at 00:38
  • @chux-ReinstateMonica Yes, so that's why I did the inline function version as a choice. – Craig Estey Mar 11 '21 at 00:41
  • @chux-ReinstateMonica As to the extra error checking, I'd leave that out of `FGETS` but one could add several different wrapper macros in the `FGETOF` vein, to do extended checking. Personally, I've rarely needed to differentiate an error from EOF for simple programs like we have. But, to do that, my preference would be to put that in the helper function rather than the macro body. For EOF, we just want to stop the loop. For an error, the remedy would depend upon the context. For most simple cases, it would be `perror` and `exit`. So, we define an extra macro implementing the policy we want. – Craig Estey Mar 11 '21 at 00:49
  • @CraigEstey Agree that a fuller input handling strains the limits of sensible macro coding. – chux - Reinstate Monica Mar 11 '21 at 00:53