In said:
You mean this case:
sizeof s = 10
012345678
input error #2: 'ERR_LENGTH'
IN: '012345678'
Yup.
Understood. The fix should not be too difficult, but I would imply a
strlen(), which is a waste of time.
What the hell do you need a strlen() for? I can't see any in your
fixed solution below!
Dou you mean I should have coded this function like a function-like macro?
It is a static function. I let the compiler optimize it. Some of them do
inline in such conditions. Don't tell me that I should have use the C99
inline feature, or worst, some gcc extension. I attempt to write today's
/portable/ code. (At work, I use at last 4 different C-compilers for various
targets like x86, 68k, PowerPC or Texas DSP).
"Inlining" means writing the code right there, instead of putting it into
a function and calling it. It is the logical equivalent of the C99 inline
keyword, but is achieved by the programmer, not by the compiler.
Here is the fix (you will like it!):
Indeed! It is a brilliant confirmation of my point about the
complexity of *properly* using fgets() for reading a line of input!
You have also confirmed my statement that most people don't get it
right at the first attempt ;-)
static int clear_in (FILE * fp)
{
int n = 0;
int c;
do
{
c = fgetc (fp);
if (c != EOF)
{
n++;
}
}
while (c != '\n' && c != EOF);
return n;
}
int fget_s (char *s, size_t size, FILE * fp)
{
int err = IO_OK;
if (s != NULL)
{
if (size > 1)
{
if (fp != NULL)
{
if (fgets (s, size, fp) != NULL)
{
char *p = strchr (s, '\n');
if (p)
{
*p = 0;
}
else
{
int n = clear_in (fp);
if (n > 1)
{
err = IO_ERR_LENGTH;
}
}
}
else
{
err = IO_ERR_READ;
}
}
else
{
err = IO_ERR_FILE;
}
}
else
{
err = IO_ERR_BUFFER_SIZE;
}
}
else
{
err = IO_ERR_BUFFER;
}
return err;
}
The indentation is supposed to be crystal clear (My DOS port of GNUIndent
1.91). If you don't like it, just reindent the code with you own settings.
(K&R-style is more compact)
The indentation is not the issue. The number of indentation levels
caused by the nested control statements is the issue. Code requiring
more than three nested control statements is badly designed and horribly
difficult to read, no matter how well indented.
Although your mama didn't tell you, C is not Pascal and there is no
point in writing Pascal code disguised as C code. I have rewritten
your fget_s without using any kind of nested control statements:
int fget_s (char *s, size_t size, FILE * fp)
{
char *p;
if (s == NULL) return IO_ERR_BUFFER;
if (size <= 1) return IO_ERR_BUFFER_SIZE;
if (fp == NULL) return IO_ERR_FILE;
if (fgets(s, size, fp) == NULL) return IO_ERR_READ;
p = strchr(s, '\n');
if (p != NULL) {
*p = 0;
return IO_OK;
}
if (clear_in(fp) == 1) return IO_OK;
return IO_ERR_LENGTH;
}
No need to figure out which else matches which if (there is no else at
all) and, by testing the right conditions, you don't need nested if's
either. If your employer forbids more than one return statement per
function, I'd suggest finding a new job.
char s[NN + 1] = "", c;
int rc = fscanf(fp, "%NN[^\n]%1[\n]", s, &c);
if (rc == 1) fscanf("%*[^\n]%*c);
if (rc == 0) getc(fp);
I agree that it's short and compact, but I'd call it cryptic.
What is the cryptic part? A simple fscanf call, followed by two simple
int rc = fscanf(fp, "%NN[^\n]%1[\n]", s, &c);
What is NN ? A macro? Are you sure a macro is replaced in a string?
Nope, it's not a macro, it's a place holder for an embedded constant.
I would have written it:
int rc = fscanf(fp, "%*[^\n]%1[\n]", NN, s, &c);
Unfortunately, this is not possible, * has different semantics in scanf
formats: it's the assignment suppression character, as exemplified in
my own code below.
if (rc == 1) fscanf("%*[^\n]%*c);
I think some parameters are missing for the two '*' and the two '%'.
Additionally, a '"' is missing too. Could you please fix that.
The only missing bit is the string literal terminator:
if (rc == 1) fscanf("%*[^\n]%*c");
You're in dire need of a clue about scanf and friends.
I feel free to call this code cryptic and hard to read and it seems that I'm
not alone.
You have amply demonstrated that you're not competent enough to make such
a statement, for the simple reason that you don't know how scanf works.
Furthermore, truth is not a democracy issue. The fscanf calls are
cryptical to my mother, too, but she doesn't have a single clue about C.
See my point?
We don't have the same definition for 'complicated'. You count the lines,
Wrong! The line count is irrelevant. The code structure is.
I try to read the code. That's the difference.
Indeed. As demonstrated above, your code is very unreadable, being very
badly structured.
Nobody win[s?]. Just a different approach.
I disagree. Take my version of fget_s, add vertical space and braces
according to your taste and it's still a lot more readable than
your version.
Dan
