Custom Scanf Routine

[Tarique]

I have tried to write my custom scanf function on the lines of minprintf
provided in K&R2.In the function myscanf() i access memory directly
using the address passed to the function .Can it be dangerous ?

I am getting the correct output though.Any help is appreciated.

/*Include Files*/

/*Assisting Functions*/
int flushln(FILE *f){ /*Code*/}
char *input(char *message){/*Code*/}
static int getInt(void){/*Code*/}


/*My Custom Scanf Routine*/

int myscanf(const char* format,...)
{
va_list ap;
const char *p;
int count = 0;
int temp;

va_start(ap,format);
for( p = format ; *p ;p++) {
if( *p != '%'){
continue;
}
switch(*++p){
case 'd':
if(temp = getInt()){
*(long *)va_arg(ap,int) = temp;/*Direct Memory Access*/
count ++;
}
else{ puts("Input Error"); }
break;
}
}
va_end(ap);
return count;
}

int main(void)
{
int a = 0,b = 0;
myscanf("%d %d",&a,&b);
printf("%d %d",a,b);

return 0;
}

Thanks!

 

[viza]

Hi

I have tried to write my custom scanf function

*(long *)va_arg(ap,int) = temp;

That is a very bad idea, it might work on certain systems where pointers
are the same size as integers, but that is often not the case.

Even if this does work once, the next operation on ap might fail.

va_arg needs to be told the type of the argument that was written in
place of the ... in when the function was called. You should always put
the correct type in the second argument. In most cases that means you
won't need to cast the return.

* va_arg(ap,long*) = temp;

will work perfectly, as long as a long pointer was really written at the
current place in the argument list.

HTH
viza

 

[Tarique]

Tarique wrote:
....snip...

This is my code for a minimal custom scanf function(for entering valid
ints n longs with error checking)
I would be really grateful if someone can review it.Comments awaited!

Thank You

#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>
#include <stdarg.h>

#define BUFFSIZE 98 + 2 /*Used by input()*/

/*Clear The Input stream if required*/
int flushln(FILE *f) {
int ch;
while (('\n' != (ch = getc( f ))) && (EOF != ch))
continue;
return ch;
}

/*Accept a string from user (parse later on)*/
char *input(const char* message,char *buff)
{
char buffer[ BUFFSIZE ];
char *p = &buffer[ BUFFSIZE-1 ];

if(message != "")
puts(message);

buffer[BUFFSIZE -1] = '$';

if( fgets(buffer,BUFFSIZE,stdin) == NULL) {
if(ferror(stdin)) {
perror("Input Stream Error");
clearerr( stdin );
return "I/O";
}
if(feof(stdin)) {
perror("EOF Encountered");
clearerr( stdin );
return "EOF";
}
}
else{
if(!((*p == '$') || ( (*p == '\0') && (*--p == '\n') ))) {
flushln(stdin);
puts("Too long input!\n\n");
return NULL;
}
}
strcpy(buff,buffer);
return buff;
}

/*Accept a string and pass a valid long int if possible,else return 0*/
long getInt(void)
{
char intbuff[100];
char *buffptr = intbuff;
char *end_ptr;
long int lVal;
int trial = 1; /*No of tries on wrong input*/
int dist=0;
errno = 0;

while(trial-- != 0)
{
buffptr = input("Enter :",intbuff);
if (buffptr == "I/O" || buffptr == "EOF" || buffptr == NULL)
return 0;

lVal= strtol(buffptr, &end_ptr, 0);
if (ERANGE == errno){
perror("Out of Range");
}
else if (lVal > INT_MAX){
perror("Too Large!");
}
else if (lVal < INT_MIN){
perror("Too Small");
}
else if (end_ptr == buffptr){
printf("Not a Valid Integer\n\n");
}
else
return lVal;
}
return 0;
}

int myscanf(const char* format,...)
{
va_list ap;
const char *p;
int count = 0;
int temp;

va_start(ap,format);
for( p = format ; *p ;p++) {
if( *p != '%'){
continue;
}
switch(*++p){
case 'd':
if(temp =(int)getInt()){
*va_arg(ap,long *) = temp;/* Direct Memory Access to MEM */
count ++;
}
else break;
break;

case 'l':
if((*++p) == 'd'){
if(temp =getInt()){
*va_arg(ap,long *) = temp;/* Direct Memory Access to MEM */
count ++;
p--;
}
else {
*va_arg(ap,long *) = 0;/*Invalid input ,so set variable to zero*/
p--;
break;
}
}
break;
default:
break;
}
}
va_end(ap);
return count;
}

int main(void)
{
long int a = 0,b = 0;
int c = 0,d = 0;

myscanf("%ld %ld",&a,&b);
printf("%ld %ld \n\n",a,b);

myscanf("%d %d",&c,&d);
printf("%d %d",c,d);

return 0;
}

 

[santosh]

Tarique wrote:
...snip...

This is my code for a minimal custom scanf function(for entering valid
ints n longs with error checking)
I would be really grateful if someone can review it.Comments awaited!

Thank You

Just a note. Haven't seen it in detail, sorry.

Also in future please use spaces in place of tabs when posting to
Usenet. As you can see below, some software on the Usenet stripped out
all your tabs, rendering unformatted code, which is very difficult to
read.

#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>
#include <stdarg.h>

#define BUFFSIZE 98 + 2 /*Used by input()*/

/*Clear The Input stream if required*/
int flushln(FILE *f) {
int ch;
while (('\n' != (ch = getc( f ))) && (EOF != ch))
continue;
return ch;
}

/*Accept a string from user (parse later on)*/
char *input(const char* message,char *buff)
{
char buffer[ BUFFSIZE ];
char *p = &buffer[ BUFFSIZE-1 ];

if(message != "")
puts(message);

buffer[BUFFSIZE -1] = '$';

if( fgets(buffer,BUFFSIZE,stdin) == NULL) {
if(ferror(stdin)) {
perror("Input Stream Error");
clearerr( stdin );
return "I/O";
}
if(feof(stdin)) {
perror("EOF Encountered");
clearerr( stdin );
return "EOF";
}
}
else{
if(!((*p == '$') || ( (*p == '\0') && (*--p == '\n') ))) {
flushln(stdin);
puts("Too long input!\n\n");
return NULL;
}
}
strcpy(buff,buffer);
return buff;
}

/*Accept a string and pass a valid long int if possible,else return
0*/ long getInt(void)
{
char intbuff[100];
char *buffptr = intbuff;
char *end_ptr;
long int lVal;
int trial = 1; /*No of tries on wrong input*/
int dist=0;
errno = 0;

while(trial-- != 0)
{
buffptr = input("Enter :",intbuff);
if (buffptr == "I/O" || buffptr == "EOF" || buffptr == NULL)
return 0;

lVal= strtol(buffptr, &end_ptr, 0);
if (ERANGE == errno){
perror("Out of Range");
}
else if (lVal > INT_MAX){
perror("Too Large!");
}
else if (lVal < INT_MIN){
perror("Too Small");
}

What will you do on implementations where INT_MAX == LONG_MAX and
INT_MIN == LONG_MIN?

<snip rest>

 

[viza]

I would be really grateful if someone can review it.Comments awaited!

#define BUFFSIZE 98 + 2 /*Used by input()*/

Generally it's not smart to use arbitrary fixed length buffers. Are you
certain that the input won't be bigger? If your code is well written
then it that would cause it to fail, and if it is not well written then
it could FEYC (f***ing explode your computer).

/*Clear The Input stream if required*/ int flushln(FILE *f) {
int ch;
while (('\n' != (ch = getc( f ))) && (EOF != ch))
continue;
return ch;
}

Gah! ugly. It's best that you don't use assignments as truth values,
especially as a beginner. That continue is superfluous too. I would
also ungetc() the character and return void.

/*Accept a string from user (parse later on)*/ char *input(const char*
message,char *buff) {
char buffer[ BUFFSIZE ];
char *p = &buffer[ BUFFSIZE-1 ];

char *p= buffer + BUFSIZE - 1; might be easier to read.

if(message != "")

You can't do that in C. Well, you can, but it doesn't mean what you
think. Lookup the strcmp() function.

puts(message);
buffer[BUFFSIZE -1] = '$';
if( fgets(buffer,BUFFSIZE,stdin) == NULL) {
if(ferror(stdin)) {
perror("Input Stream Error");
clearerr( stdin );
return "I/O";

It's not usual to return strings like that one. In case of error perhaps
you might want to return NULL.

}
if(feof(stdin)) {
perror("EOF Encountered");
clearerr( stdin );
return "EOF";
}
}
else{
if(!((*p == '$') || ( (*p == '\0') && (*--p == '\n') ))) {
flushln(stdin);
puts("Too long input!\n\n");
return NULL;
}
}
strcpy(buff,buffer);

If you know that buff is at least as big as buffer, why do you need
buffer in the first place? Read to buff.

return buff;
}

/*Accept a string and pass a valid long int if possible,else return 0*/
long getInt(void)
{
char intbuff[100];
char *buffptr = intbuff;
char *end_ptr;
long int lVal;
int trial = 1; /*No of tries on wrong input*/ int dist=0;
errno = 0;

while(trial-- != 0)
{
buffptr = input("Enter :",intbuff);
if (buffptr == "I/O" || buffptr == "EOF" || buffptr == NULL)
return 0;

lVal= strtol(buffptr, &end_ptr, 0);
if (ERANGE == errno){

You didn't set errno to zero. Well you did, but it might have changed
many times since then. Do it immediately before each conversion.

I haven't gome through it all, but HTH
viza

 

[Tarique]

viza wrote:
...snip...

I haven't gome through it all, but HTH
viza

@santosh and Viza

Thanks for the quick review.Ive pasted the code here :
http://phpfi.com/329176
Hope it is easier to read.

 

[Barry Schwarz]

This is my code for a minimal custom scanf function(for entering valid
ints n longs with error checking)
I would be really grateful if someone can review it.Comments awaited!

Thank You

#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>
#include <stdarg.h>

#define BUFFSIZE 98 + 2 /*Used by input()*/

/*Clear The Input stream if required*/
int flushln(FILE *f) {
int ch;
while (('\n' != (ch = getc( f ))) && (EOF != ch))
continue;
return ch;
}

/*Accept a string from user (parse later on)*/
char *input(const char* message,char *buff)
{
char buffer[ BUFFSIZE ];
char *p = &buffer[ BUFFSIZE-1 ];

if(message != "")
puts(message);

buffer[BUFFSIZE -1] = '$';

if( fgets(buffer,BUFFSIZE,stdin) == NULL) {
if(ferror(stdin)) {
perror("Input Stream Error");
clearerr( stdin );
return "I/O";
}
if(feof(stdin)) {
perror("EOF Encountered");
clearerr( stdin );
return "EOF";
}
}
else{
if(!((*p == '$') || ( (*p == '\0') && (*--p == '\n') ))) {

Please don't use tabs, especially within a line.

This test is backwards. If *p is '$', then a short line was entered
and there is nothing to flush. If the second half is true, exactly
BUFFSIZE-1 characters were entered (including the Enter key) and there
is still nothing to flush.

An easier test would be to set buffer[BUFFSIZE-2] to '\n'. If it is a
different non-zero value after the fgets, then too much data was
entered.

flushln(stdin);
puts("Too long input!\n\n");
return NULL;
}
}
strcpy(buff,buffer);
return buff;
}

/*Accept a string and pass a valid long int if possible,else return 0*/
long getInt(void)
{
char intbuff[100];
char *buffptr = intbuff;
char *end_ptr;
long int lVal;
int trial = 1; /*No of tries on wrong input*/
int dist=0;
errno = 0;

while(trial-- != 0)
{
buffptr = input("Enter :",intbuff);
if (buffptr == "I/O" || buffptr == "EOF" || buffptr == NULL)
return 0;

lVal= strtol(buffptr, &end_ptr, 0);

You need to indent consistently. This statement is not part of the
range of the preceding if. It should be indented to the same level as
that if.

if (ERANGE == errno){

You should reset errno before calling strtol so that you don't process
residual data. strtol is not allowed to reset errno so you must do
it.

perror("Out of Range");
}
else if (lVal > INT_MAX){
perror("Too Large!");
}
else if (lVal < INT_MIN){
perror("Too Small");
}
else if (end_ptr == buffptr){
printf("Not a Valid Integer\n\n");

This is insufficient. Input of the form "123abc" will appear valid.
To be sure that strtol processed all the characters, make sure end_ptr
points to either '\n' or '\0'.

}
else
return lVal;

How do you distinguish between input of 0 and one of the "error"
conditions such as "I/O"?

}
return 0;
}

int myscanf(const char* format,...)
{
va_list ap;
const char *p;
int count = 0;
int temp;

va_start(ap,format);
for( p = format ; *p ;p++) {
if( *p != '%'){
continue;
}
switch(*++p){

These two statements are within the range of the for but you indenting
makes them look independent.

case 'd':
if(temp =(int)getInt()){

The cast is not needed.

Why are you using %d to describe a long and confuse everyone.

Why do you not accept 0 as valid input?

*va_arg(ap,long *) = temp;/* Direct Memory Access to MEM */
count ++;
}
else break;

This is superfluous. If you remove it, the code will behave the same.

break;

case 'l':
if((*++p) == 'd'){
if(temp =getInt()){
*va_arg(ap,long *) = temp;/* Direct Memory Access to MEM */
count ++;
p--;
}
else {
*va_arg(ap,long *) = 0;/*Invalid input ,so set variable to zero*/

I still don't understand why 0 is illegal but it sure seems like a
deliberate decision.

p--;
break;
}
}
break;
default:
break;
}

If the value processed for %d is 0, you do not store any value for the
corresponding argument AND you do not pop the argument off the list.

If the value processed for %ld is 0, you store 0 and pop the argument
but don't increment count.

There could be an inconsistency between the value of count returned
and which arguments contain valid data.

}
va_end(ap);
return count;
}

int main(void)
{
long int a = 0,b = 0;
int c = 0,d = 0;

myscanf("%ld %ld",&a,&b);
printf("%ld %ld \n\n",a,b);

myscanf("%d %d",&c,&d);

Here you invoke undefined behavior. myscanf will treat &c and &d as
long int* when they aren't. I think you meant for myscanf to extract
an int*, not a long*, for the %d case.

printf("%d %d",c,d);

return 0;
}

Remove del for email

 

[CBFalconer]

Tarique wrote:
.... snip ...


Gah! ugly. It's best that you don't use assignments as truth
values, especially as a beginner. That continue is superfluous
too. I would also ungetc() the character and return void.

Not in the least ugly. Apart from the awkward location of the
initial comment. Works like a charm, too. The continue prevents
misreading an isolated semi. Your ungetc recommendation would make
the routine fail to perform the function indicated by the name.

 

[Barry Schwarz]

This is my code for a minimal custom scanf function(for entering valid
ints n longs with error checking)
I would be really grateful if someone can review it.Comments awaited!

Thank You

#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>
#include <stdarg.h>

#define BUFFSIZE 98 + 2 /*Used by input()*/

/*Clear The Input stream if required*/
int flushln(FILE *f) {
int ch;
while (('\n' != (ch = getc( f ))) && (EOF != ch))
continue;
return ch;
}

/*Accept a string from user (parse later on)*/
char *input(const char* message,char *buff)
{
char buffer[ BUFFSIZE ];
char *p = &buffer[ BUFFSIZE-1 ];

if(message != "")
puts(message);

buffer[BUFFSIZE -1] = '$';

if( fgets(buffer,BUFFSIZE,stdin) == NULL) {
if(ferror(stdin)) {
perror("Input Stream Error");
clearerr( stdin );
return "I/O";
}
if(feof(stdin)) {
perror("EOF Encountered");
clearerr( stdin );
return "EOF";
}
}
else{
if(!((*p == '$') || ( (*p == '\0') && (*--p == '\n') ))) {

Please don't use tabs, especially within a line.

This test is backwards.

Obviously I missed the !.


Remove del for email

 

[viza]

Not in the least ugly.

I wouldn't take it home.

Apart from the awkward location of the initial comment. Works like a
charm, too. The continue prevents misreading an isolated semi.

Your ungetc recommendation would make the routine fail to perform the
function indicated by the name.

True. I misread the ugly loop construct and thought it read one-past the
newline.

 

[Tarique]

....snip..


Here you invoke undefined behavior. myscanf will treat &c and &d as
long int* when they aren't. I think you meant for myscanf to extract
an int*, not a long*, for the %d case.

Yes that is what i want it to do ! But why does that invoke UB ? I
thought the cast should produce correct result !!

Secondly Santosh had asked "What will you do on implementations where
INT_MAX == LONG_MAX and INT_MIN == LONG_MIN? "
On my system i have INT_MAX == LONG_MAX and INT_MIN == LONG_MIN
and the program seems to produce correct results.Am I missing something
there?

Well I,ve made the changes suggested and this the modified one . This
one produces results more consistently as expected(at least it seems to me!)

#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>
#include <stdarg.h>

#define BUFFSIZE 98 + 2


int flushln(FILE *f) { /*Clear The Input stream if required*/
int ch;
while (('\n' != (ch = getc( f ))) && (EOF != ch))
;
return ch;
}


/*Accept a string from user*/

char *input(const char* message,char *buff,size_t buff_len)
{
char buffer[ BUFFSIZE ];
char *p = buffer + BUFFSIZE - 1;

if(strcmp(message,""))
puts(message);

buffer[BUFFSIZE -1] = '$';

if( fgets(buffer,BUFFSIZE,stdin) == NULL) {
if(ferror(stdin)) {
perror("Input Stream Error");
clearerr( stdin );
return "I/O";
}
else if(feof(stdin)) {
perror("EOF Encountered");
clearerr( stdin );
return "EOF";
}
}
else{
if(!((*p == '$') || ( (*p == '\0') && (*--p == '\n') ))) {
flushln(stdin);
puts("Too long input!\n\n");
return NULL;
}
}

if(buff_len >= strlen(buffer)){ /*Input longer than the dest buff can hold*/
strcpy(buff,buffer);
return buff;
}
else
return NULL;
}

/*Accept a string and pass a valid long int if possible,else return 0*/
long getInt(void)
{
char intbuff[20];
char *buffptr = intbuff;
char *end_ptr = "\n";
long int lVal;
int trial = 1; /*No of trials allowed on error */
int dist=0;
errno = 0;

while(trial-- != 0)
{
buffptr = input("Enter :", intbuff, 19);

if(!strcmp(intbuff,"I/O")){
fprintf(stderr,"I/O Errro\n");
return -1;
}
else if (!strcmp(intbuff,"EOF")){
fprintf(stderr,"EOF\n");
return -1;
}
else if(buffptr == NULL){
fprintf(stderr,"Too Long Input\n");
return -1;
}

errno = 0;
lVal= strtol(buffptr, &end_ptr, 0);


if (ERANGE == errno){
perror("Out of Range");
}
else if (lVal > LONG_MAX){
perror("Too Large!");
}
else if (lVal < LONG_MIN){
perror("Too Small");
}
else if (end_ptr == buffptr){
printf("Not a Valid Integer\n\n");
}
else
return lVal;
}
return -1;
}

int myscanf(const char* format,...)
{
va_list ap;
const char *p;
int count = 0;
int temp = 0 ;

va_start(ap,format);
for( p = format ; *p ;p++) {
if( *p != '%'){
continue;
}
switch(*++p){
case 'd':
if(temp = (int)getInt()){
if(temp == 0)
*va_arg(ap,long *) = 0;/*Ugly Hack ?*/
*va_arg(ap,long *) = temp;/* Direct Memory Access*/
count ++;
}
else {
*va_arg(ap,long *) = 0;
p--;
break;
}

case 'l':
if((*++p) == 'd'){
if(temp = getInt()){
if(temp == 0)
*va_arg(ap,long *) = 0;/*Ugly Hack!*/
*va_arg(ap,long *) = temp;
count ++;
p--;
}
else {
*va_arg(ap,long *) = 0;
p--;
break;
}
}
default:
break;
}
}
va_end(ap);
return count;
}

int main(void)
{
long int a = 0,b = 0;
int c = 0,d = 0;

myscanf("%ld %ld",&a,&b);
printf("%ld %ld \n\n",a,b);

myscanf("%d %d",&c,&d);
printf("%d %d",c,d);

return 0;
}


Thanks for the suggestions.

Tarique

 

[Barry Schwarz]

Yes that is what i want it to do ! But why does that invoke UB ? I
thought the cast should produce correct result !!

There is no cast in the statement under discussion. &c is obviously
an int*. Your code in myscanf use the va_arg macro with an argument
of long*.

If the two types don't have the same size, va_arg will extract
the wrong number of bytes from the "argument list".
If an int* and long* are passed by different mechanisms (e.g.,
different registers), it may not even find the argument.
If the two pointer types don't have the same representation, the
address will be misinterpreted.
myscanf dereferences the argument on the left side of an
assignment. The address of the int could be not properly aligned for
a long.
If sizeof(long)>sizeof(int), you attempt to store too many bytes
in the int.

Each of these errors invokes undefined behavior.



Remove del for email

 

[Barry Schwarz]

snip

Well I,ve made the changes suggested and this the modified one . This
one produces results more consistently as expected(at least it seems to me!)

Actually it is considerably worse.

#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>
#include <stdarg.h>

#define BUFFSIZE 98 + 2


int flushln(FILE *f) { /*Clear The Input stream if required*/
int ch;
while (('\n' != (ch = getc( f ))) && (EOF != ch))
;
return ch;

You are still not indenting with any consistency. The return is not
part of the while. You have the same problem in getInt.

}


/*Accept a string from user*/

char *input(const char* message,char *buff,size_t buff_len)
{
char buffer[ BUFFSIZE ];
char *p = buffer + BUFFSIZE - 1;

if(strcmp(message,""))
puts(message);

buffer[BUFFSIZE -1] = '$';

if( fgets(buffer,BUFFSIZE,stdin) == NULL) {
if(ferror(stdin)) {
perror("Input Stream Error");
clearerr( stdin );
return "I/O";
}
else if(feof(stdin)) {
perror("EOF Encountered");
clearerr( stdin );
return "EOF";
}
}
else{
if(!((*p == '$') || ( (*p == '\0') && (*--p == '\n') ))) {
flushln(stdin);
puts("Too long input!\n\n");
return NULL;
}
}

if(buff_len >= strlen(buffer)){ /*Input longer than the dest buff can hold*/
strcpy(buff,buffer);
return buff;
}
else
return NULL;
}

/*Accept a string and pass a valid long int if possible,else return 0*/
long getInt(void)
{
char intbuff[20];
char *buffptr = intbuff;
char *end_ptr = "\n";
long int lVal;
int trial = 1; /*No of trials allowed on error */
int dist=0;
errno = 0;

while(trial-- != 0)
{
buffptr = input("Enter :", intbuff, 19);

if(!strcmp(intbuff,"I/O")){
fprintf(stderr,"I/O Errro\n");
return -1;
}
else if (!strcmp(intbuff,"EOF")){
fprintf(stderr,"EOF\n");
return -1;
}
else if(buffptr == NULL){
fprintf(stderr,"Too Long Input\n");
return -1;
}

errno = 0;
lVal= strtol(buffptr, &end_ptr, 0);

If the user enters a number with a leading 0, it will be treated as
octal. Is that what you want?

if (ERANGE == errno){
perror("Out of Range");
}
else if (lVal > LONG_MAX){

What makes you think strtol can ever return a value greater than
LONG_MAX?

perror("Too Large!");
}
else if (lVal < LONG_MIN){

Or less than LONG_MIN?

perror("Too Small");
}
else if (end_ptr == buffptr){

You still do not catch the error on input of "123abc".

printf("Not a Valid Integer\n\n");
}
else
return lVal;
}
return -1;

So you have decided that -1 will never be an acceptable input.

}

int myscanf(const char* format,...)
{
va_list ap;
const char *p;
int count = 0;
int temp = 0 ;

va_start(ap,format);
for( p = format ; *p ;p++) {
if( *p != '%'){
continue;
}
switch(*++p){

And here you have messed up the indenting even worse. It looks as if
the for loop terminates before the switch statement executes.

case 'd':
if(temp = (int)getInt()){
if(temp == 0)

If you reach this statement, can it ever evaluate to true?

*va_arg(ap,long *) = 0;/*Ugly Hack ?*/

Why do you think these two lines are needed? If temp is 0, what do
they do that the next line does not?

If temp is 0, you store 0 into two different objects, one in the above
statement and one in the following statement. Is that what you really
want to do?

*va_arg(ap,long *) = temp;/* Direct Memory Access*/

The term Direct Memory Access has a special meaning in some hardware
designs. You are simply dereferencing a pointer.

count ++;

There is no break here. You will fall into the 'l' case.

}
else {
*va_arg(ap,long *) = 0;
p--;

Why are you backing up to the % symbol?

break;
}

case 'l':
if((*++p) == 'd'){
if(temp = getInt()){

Why do you care if the input value is 0?

if(temp == 0)
*va_arg(ap,long *) = 0;/*Ugly Hack!*/
*va_arg(ap,long *) = temp;
count ++;
p--;

Why are you backing up to the l?

}
else {
*va_arg(ap,long *) = 0;
p--;
break;
}
}
default:
break;
}
}
va_end(ap);
return count;
}

int main(void)
{
long int a = 0,b = 0;
int c = 0,d = 0;

myscanf("%ld %ld",&a,&b);
printf("%ld %ld \n\n",a,b);

myscanf("%d %d",&c,&d);

The undefined behavior is still here since myscan only processes long*
but you are passing int*.

printf("%d %d",c,d);

return 0;
}

Remove del for email

 

[Tarique]

snip


Actually it is considerably worse.

Just could not believe that i had actually messed it up completely!
One more attempt ..phew!

I've re-typed the entire code and hope that the indentation doesn't get
messed up on usenet.This is the default indentation produced by MSVC++
08.I tried reducing it in the last post and the result was far worse!
If it still does, please read it here : http://phpfi.com/329403

Thanks again for all the patience


#include <stdio.h>
#include <errno.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>
#include <stdarg.h>

#define BUFFSIZE 98 + 2

int flushln(FILE *f)
{
int ch;
while(('\n' != (ch = getc( f ))) && (EOF != ch))
;
return ch;
}

char* input(const char* message,char* buff,size_t buff_len)
{
char buffer[ BUFFSIZE ];
char* p = buffer + BUFFSIZE - 1;

if( strcmp( message, "" ) )
puts( message );

buffer[ BUFFSIZE - 1 ] = '$';

if( fgets( buffer, BUFFSIZE, stdin) == NULL ) {
if( ferror(stdin) ) {
perror( "Input Stream Error" );
clearerr( stdin );
return "I/O";
}
else if( feof(stdin) ) {
perror( "Input Stream Error" );
clearerr( stdin );
return "EOF";
}
}

else {
if(!((*p == '$') || ((*p == '\0') && (*--p == '\n')))) {
flushln( stdin );
return NULL ;
}
}

if( buff_len >= strlen(buffer) ) {
strcpy( buff, buffer );
return buff;
}
else {
return NULL;
}
}


long getInt( int* err_flag )
{
char intbuff[21] = {0} ;
char* buffptr = intbuff;
char* end_ptr = NULL;
long int lval = 0;
int trial = 1;
errno = 0;

while( trial-- )
{
buffptr = input( "Enter :", intbuff, 20 );

if( !strcmp( intbuff , "I/O" ) ) {
fprintf( stderr, "I/O Error \n" );
*err_flag = 0;
return -1;
}
else if( !strcmp( intbuff , "EOF" ) ) {
fprintf( stderr, "EOF \n" );
*err_flag = 0;
return -1;
}
else if( buffptr == NULL ) {
fprintf( stderr, "Too Long Input\n" );
*err_flag = 0;
return -1;
}

errno = 0;
lval = strtol( buffptr, &end_ptr, 10 );

if( ERANGE == errno ) {
*err_flag = 0;
perror( " Out of Range " );
}
else if ( lval > INT_MAX ) {
*err_flag = 0;
perror( "Too Large " );
}
else if ( lval < INT_MIN ) {
*err_flag = 0;
perror( "Too Small" );
}
else if (!((*end_ptr == '\n') || (*end_ptr == '\0'))) {
*err_flag = 0;
fprintf( stderr, "Not a Valid Integer\n" );
}
else {
return lval;
}
}

*err_flag = 0;
return -1;
}


int myscanf( const char* format, ... )
{
va_list ap;
const char* p;
int count = 0;
int temp = 0;
int flag = 1;

va_start( ap, format );

for( p = format ; *p ; p++ ) {
if( *p != '%' ) {
continue;
}
switch( *++p ) {

case 'd' :
temp = (int)getInt( &flag );
if( flag ) {
*va_arg( ap, int* ) = temp;
count ++ ;
}
else {
*va_arg( ap, int* ) = 0;
flag = 1;
}

break;

case 'l':
if( (*++p) == 'd' ) {
temp = getInt( &flag );
if( flag ) {
*va_arg( ap, long * ) = temp;
count ++;
}
else {
*va_arg( ap, long * ) = 0;
flag = 1;
}
}

break;

default :
break;
}
}
va_end(ap);
return count;
}

int main(void)
{
long int a = 0, b = 0;
int c = 0, d = 0;

myscanf("%ld %ld",&a,&b);
printf("%ld %ld \n\n",a,b);

myscanf("%d %d",&c,&d);
printf("%d %d",c,d);

return 0;
}

Tarique

 

[Keith Thompson]

Just could not believe that i had actually messed it up completely!
One more attempt ..phew!

I've re-typed the entire code and hope that the indentation doesn't
get messed up on usenet.This is the default indentation produced by
MSVC++ 08.I tried reducing it in the last post and the result was far
worse!
If it still does, please read it here : http://phpfi.com/329403

Thanks again for all the patience

[...]

I haven't read the entire program, just bits and piece of it. At
least for now, all I have to offer is a few style points.

In flushln(), you use the (rather odd IMHO) idiom of "constant !=
expression":

while(('\n' != (ch = getc( f ))) && (EOF != ch))

In other places, you put the constant on the right hand side of the
comparison:

if(!((*p == '$') || ((*p == '\0') && (*--p == '\n')))) {

The usual reason to put the constant on the left is to avoid the error
of writing "=" rather than "==". Some programmers extend this to
operators other than "==". This:

if (x = 42) { ... }

is perfectly legal, but it assigns 42 to x rather than comparing x to
42. Some programmers cultivate the habit of writing:

if (42 == x) { ... }

so that, if they accidentally write "=" rather than "==", the
compiler will catch the error. You do this in some places, but not in
others.

My own opinion is that "if (42 == x)" is just ugly, and I never use it
myself; I find "if (x == 42") much more natural. But if you're going
to use it you should probably do so consistently. Or you can just be
careful not to write "=" when you mean "==".

Your function getInt() returns a result of type long. You call it
twice; both times, you assign the result to a variable of type int.
(In one you use an unnecessary cast; in the other you don't.) Why not
just declare getInt() to return an int? Or declare temp as a long?
Often (but by no means always), the need to convert something from one
type to another means that both things should have been of the same
type in the first place.

I stumbled over your invocations of va_arg, such as:

*va_arg( ap, int* ) = temp;

va_arg() is required to be a macro, and va_arg(ap, int*) expands to an
expression of type int*, so this seems to be valid.

But note that an invocation of va_arg *looks like* a function call.
If it really were, then the unary "*" operator would apply to the name
"va_arg", not to the result of the call, and the assignment would be
invalid.

So the assignment is valid, but it looks wrong. I'd definitely add
parentheses:

*(va_arg(ap, int*)) = temp;

 

[Ben Bacarisse]

Just could not believe that i had actually messed it up completely!
One more attempt ..phew!

I've re-typed the entire code and hope that the indentation doesn't
get messed up on usenet.This is the default indentation produced by
MSVC++ 08.I tried reducing it in the last post and the result was far
worse!
If it still does, please read it here : http://phpfi.com/329403

Thanks again for all the patience

A few things that occur to me...

#include <stdio.h>
#include <errno.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>
#include <stdarg.h>

#define BUFFSIZE 98 + 2

(98 + 2) or you might get a surprise.

int flushln(FILE *f)
{
int ch;
while(('\n' != (ch = getc( f ))) && (EOF != ch))
;
return ch;
}

char* input(const char* message,char* buff,size_t buff_len)
{
char buffer[ BUFFSIZE ];
char* p = buffer + BUFFSIZE - 1;

if( strcmp( message, "" ) )
puts( message );

buffer[ BUFFSIZE - 1 ] = '$';

Having set p, I'd write *p = '$'; It makes the test latter more
obvious. See also below for some an alternative.

if( fgets( buffer, BUFFSIZE, stdin) == NULL ) {
if( ferror(stdin) ) {
perror( "Input Stream Error" );
clearerr( stdin );
return "I/O";

I fond this odd -- returning a string to indicate an error. An enum
or a number if usually easier.

}
else if( feof(stdin) ) {
perror( "Input Stream Error" );
clearerr( stdin );
return "EOF";
}
}

else {
if(!((*p == '$') || ((*p == '\0') && (*--p == '\n'))))

I'd write p[-1] == '\n' since you don't need p to change. It is
clearer that an expression with a side-effect. My preference, since
the '$' is entirely arbitrary, is to set *p = !0; above. All you care
about is that it is some value other than zero and this makes that
clear. Your test here is then simplified to !(*p || p[-1] == '\n').

{
flushln( stdin );
return NULL ;
}
}

if( buff_len >= strlen(buffer) ) {
strcpy( buff, buffer );
return buff;
}
else {
return NULL;
}
}


long getInt( int* err_flag )
{
char intbuff[21] = {0} ;
char* buffptr = intbuff;
char* end_ptr = NULL;
long int lval = 0;
int trial = 1;
errno = 0;

while( trial-- )
{
buffptr = input( "Enter :", intbuff, 20 );

if( !strcmp( intbuff , "I/O" ) ) {

I think this is wrong. You return "I/O" rather than copy into the
array passed to input.

fprintf( stderr, "I/O Error \n" );
*err_flag = 0;
return -1;
}
else if( !strcmp( intbuff , "EOF" ) ) {
Ditto.

fprintf( stderr, "EOF \n" );
*err_flag = 0;
return -1;
}
else if( buffptr == NULL ) {
fprintf( stderr, "Too Long Input\n" );
*err_flag = 0;
return -1;
}

errno = 0;
lval = strtol( buffptr, &end_ptr, 10 );

if( ERANGE == errno ) {
*err_flag = 0;
perror( " Out of Range " );
}
else if ( lval > INT_MAX ) {
*err_flag = 0;
perror( "Too Large " );
}
else if ( lval < INT_MIN ) {
*err_flag = 0;
perror( "Too Small" );
}
else if (!((*end_ptr == '\n') || (*end_ptr == '\0')))
{

This seems a bit harsh! Do you want to reject " 123 " as an integer?

*err_flag = 0;
fprintf( stderr, "Not a Valid Integer\n" );
}
else {
return lval;
}
}

*err_flag = 0;
return -1;
}


int myscanf( const char* format, ... )
{
va_list ap;
const char* p;
int count = 0;
int temp = 0;
int flag = 1;

Yucky name. You one that tell the reader what is signified by a true
value. If you can't (as I think is the case here because you use it
for lots of things, then it is probably better to redesign that).

va_start( ap, format );

for( p = format ; *p ; p++ ) {
if( *p != '%' ) {
continue;
}

strchr is simpler though you have to test the result instead of
relying on the default case of the switch.

 

[Barry Schwarz]

snip


Actually it is considerably worse.

Just could not believe that i had actually messed it up completely!
One more attempt ..phew!

I've re-typed the entire code and hope that the indentation doesn't get
messed up on usenet.This is the default indentation produced by MSVC++
08.I tried reducing it in the last post and the result was far worse!
If it still does, please read it here : http://phpfi.com/329403

Thanks again for all the patience


#include <stdio.h>
#include <errno.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>
#include <stdarg.h>

#define BUFFSIZE 98 + 2

int flushln(FILE *f)
{
int ch;
while(('\n' != (ch = getc( f ))) && (EOF != ch))
;
return ch;
}

char* input(const char* message,char* buff,size_t buff_len)
{
char buffer[ BUFFSIZE ];
char* p = buffer + BUFFSIZE - 1;

if( strcmp( message, "" ) )
puts( message );

buffer[ BUFFSIZE - 1 ] = '$';

if( fgets( buffer, BUFFSIZE, stdin) == NULL ) {
if( ferror(stdin) ) {
perror( "Input Stream Error" );
clearerr( stdin );
return "I/O";
}
else if( feof(stdin) ) {
perror( "Input Stream Error" );
clearerr( stdin );
return "EOF";
}
}

else {
if(!((*p == '$') || ((*p == '\0') && (*--p == '\n')))) {
flushln( stdin );
return NULL ;
}
}

if( buff_len >= strlen(buffer) ) {
strcpy( buff, buffer );
return buff;
}
else {
return NULL;
}
}


long getInt( int* err_flag )
{
char intbuff[21] = {0} ;
char* buffptr = intbuff;
char* end_ptr = NULL;
long int lval = 0;
int trial = 1;
errno = 0;

while( trial-- )
{
buffptr = input( "Enter :", intbuff, 20 );

if( !strcmp( intbuff , "I/O" ) ) {
fprintf( stderr, "I/O Error \n" );
*err_flag = 0;
return -1;
}
else if( !strcmp( intbuff , "EOF" ) ) {
fprintf( stderr, "EOF \n" );
*err_flag = 0;
return -1;
}
else if( buffptr == NULL ) {
fprintf( stderr, "Too Long Input\n" );
*err_flag = 0;
return -1;
}

errno = 0;
lval = strtol( buffptr, &end_ptr, 10 );

if( ERANGE == errno ) {
*err_flag = 0;
perror( " Out of Range " );

perror will print two messages, the one you provided and the one
corresponding to ERANGE. While it is implementation dependent, I'll
bet you cannot find an implementation where they don't say the same
thing.

}
else if ( lval > INT_MAX ) {
*err_flag = 0;
perror( "Too Large " );

At this point we know errno is 0. I have no idea what the
"corresponding message" will be but it could be something like "no
error detected". This will then be followed by your message saying
"yes there was". It is not nice to confuse the poor user.

}
else if ( lval < INT_MIN ) {
*err_flag = 0;
perror( "Too Small" );
}
else if (!((*end_ptr == '\n') || (*end_ptr == '\0'))) {
*err_flag = 0;
fprintf( stderr, "Not a Valid Integer\n" );

I would use fprintf in place of the previous calls to perror.

}
else {
return lval;
}
}

*err_flag = 0;
return -1;
}

This function cannot return a non-int value, even though it is
packaged inside a long.

int myscanf( const char* format, ... )
{
va_list ap;
const char* p;
int count = 0;
int temp = 0;
int flag = 1;

va_start( ap, format );

for( p = format ; *p ; p++ ) {
if( *p != '%' ) {
continue;
}
switch( *++p ) {

case 'd' :
temp = (int)getInt( &flag );
if( flag ) {
*va_arg( ap, int* ) = temp;
count ++ ;
}
else {
*va_arg( ap, int* ) = 0;
flag = 1;
}

break;

case 'l':
if( (*++p) == 'd' ) {
temp = getInt( &flag );
if( flag ) {
*va_arg( ap, long * ) = temp;

You are processing %ld and treating it as a specification for long.
Unfortunately, getInt will only return int values. You can store the
value in a long but you can never process a "true" long.

count ++;
}
else {
*va_arg( ap, long * ) = 0;
flag = 1;
}
}

break;

default :
break;
}
}
va_end(ap);
return count;
}

int main(void)
{
long int a = 0, b = 0;
int c = 0, d = 0;

myscanf("%ld %ld",&a,&b);
printf("%ld %ld \n\n",a,b);

myscanf("%d %d",&c,&d);
printf("%d %d",c,d);

return 0;
}

Tarique

Remove del for email

 

[Tarique]

..snip...

I would use fprintf in place of the previous calls to perror.


This function cannot return a non-int value, even though it is
packaged inside a long.

I didn't understand this ?
long int temp_long = 0; /*Added*/

...snip...

Changed this to temp_long = getInt ( &flag );
And this to *(va_arg( ap, long* )) = temp_long;

You are processing %ld and treating it as a specification for long.
Unfortunately, getInt will only return int values. You can store the
value in a long but you can never process a "true" long.

I believe i am right this time.I completely missed it the previous
time.Incorporated the above changes suggested by all.

Thanks
Tarique

 

[CBFalconer]

.... snip ...

I believe i am right this time.I completely missed it the previous
time.Incorporated the above changes suggested by all.

In English, a period is normally followed by at least one space,
usually two. I see no time structure in your code, especially none
with the fields time.I and time.Incorporated.

 

[Barry Schwarz]

Barry Schwarz wrote:

I didn't understand this ?

The function is named getInt. It returns a long. (This in itself is
a little confusing but there are several library functions that do
something similar such as getchar. However, when they do it there is
a reason.)

Before your function returns, it compares the return value to both
INT_MAX and INT_MIN and returns an error condition (-1 in your latest
iteration) if the value is outside those limits.

Therefore, even though the function returns a value of type long, the
value itself is always in the range of int. There is nothing wrong
with this in the abstract but you attempt to process longs in myscanf.
myscanf calls getInt. This is a serious lack of consistency that
would cause serious problems for users of the functions that really
want to process longs.

long int temp_long = 0; /*Added*/

...snip...


Changed this to temp_long = getInt ( &flag );

This doesn't change anything. getInt still refuses to return any
value outside the range of an int.

And this to *(va_arg( ap, long* )) = temp_long;

When you fix getInt, this will help.

I believe i am right this time.I completely missed it the previous
time.Incorporated the above changes suggested by all.

Sorry but you fixed only half the problem.


Remove del for email