A situation

  • Thread starter Christopher Benson-Manica
  • Start date
C

Christopher Benson-Manica

(I did not write this code!)

const char *GetSortMethod( unsigned int sortMeth )
{
const char *sortingstring;
if( sortMeth == 0 ) {
sortingstring = "D";
}
if( sortMeth == 1 ) {
sortingstring = "A";
}
if( sortMeth == -1 ) {
sortingstring = "";
}
return( sortingstring );
}

1) Returning an automatic pointer to a string literal is a big
mistake, correct?
2) If sortMeth isn't -1, 0, or 1, the value returned i
even less determinate than it was before...
3) sortMeth is *never* -1, since it's unsigned...
4) How about the following correction?

char GetSortMethod( int sortMeth )
{
return( !sortMeth ? 'D' : sortMeth == 1 ? 'A' : 0 );
}

5) No, just kidding! Seriously this time:

char GetSortMethod( int sortMeth )
{
if( sortMeth == 0 )
return 'D';
if( sortMeth == 1 )
return 'A';
return '\0';
}

6) The original function was used like this:

printf( "somevar='%s'\n", GetSortMethod(anum) ); /* int anum; */

Is this the best way to use the correct version?

char meth[2];
meth[0]=GetSortMethod( anum );
meth[1]='\0';

printf( "somevar='%s'\n", meth );

<OT>
7) Is the original function implementation any more viable when
compiled by a C++ compiler (namely, Borland C++ Builder 4.0)?

8) Is there any conceivable circumstance where the original function
implementation could have been correct? Even if it's really a C++
class member function?
</OT>

9) Any other suggestions or nitpicks?
 
J

Joona I Palaste

Christopher Benson-Manica said:
(I did not write this code!)
const char *GetSortMethod( unsigned int sortMeth )
{
const char *sortingstring;
if( sortMeth == 0 ) {
sortingstring = "D";
}
if( sortMeth == 1 ) {
sortingstring = "A";
}
if( sortMeth == -1 ) {
sortingstring = "";
}
return( sortingstring );
}
1) Returning an automatic pointer to a string literal is a big
mistake, correct?

No. String literals have storage that survives the execution of the
whole program. The above code is no less safe than:
int GetInt(void) {
int i;
i=1;
return i;
}
2) If sortMeth isn't -1, 0, or 1, the value returned i
even less determinate than it was before...

Well, it's indeterminate. It makes no sense to speak of the return value
before the return of the function.
3) sortMeth is *never* -1, since it's unsigned...
Right.

4) How about the following correction?
char GetSortMethod( int sortMeth )
{
return( !sortMeth ? 'D' : sortMeth == 1 ? 'A' : 0 );
}
5) No, just kidding! Seriously this time:
char GetSortMethod( int sortMeth )
{
if( sortMeth == 0 )
return 'D';
if( sortMeth == 1 )
return 'A';
return '\0';
}

That would work.
6) The original function was used like this:
printf( "somevar='%s'\n", GetSortMethod(anum) ); /* int anum; */
Is this the best way to use the correct version?
char meth[2];
meth[0]=GetSortMethod( anum );
meth[1]='\0';

printf( "somevar='%s'\n", meth );

Why not simply this?

printf("somevar='%c'\n", GetSortMethod(anum));
9) Any other suggestions or nitpicks?

None I can think of.
 
J

James Beck

(I did not write this code!)

const char *GetSortMethod( unsigned int sortMeth )
{
const char *sortingstring;
if( sortMeth == 0 ) {
sortingstring = "D";
}
if( sortMeth == 1 ) {
sortingstring = "A";
}
if( sortMeth == -1 ) {
sortingstring = "";
}
return( sortingstring );
}

1) Returning an automatic pointer to a string literal is a big
mistake, correct?
2) If sortMeth isn't -1, 0, or 1, the value returned i
even less determinate than it was before...
3) sortMeth is *never* -1, since it's unsigned...
4) How about the following correction?

char GetSortMethod( int sortMeth )
{
return( !sortMeth ? 'D' : sortMeth == 1 ? 'A' : 0 );
}

5) No, just kidding! Seriously this time:

char GetSortMethod( int sortMeth )
{
if( sortMeth == 0 )
return 'D';
if( sortMeth == 1 )
return 'A';
return '\0';
}

6) The original function was used like this:

printf( "somevar='%s'\n", GetSortMethod(anum) ); /* int anum; */

Is this the best way to use the correct version?

char meth[2];
meth[0]=GetSortMethod( anum );
meth[1]='\0';

printf( "somevar='%s'\n", meth );

<OT>
7) Is the original function implementation any more viable when
compiled by a C++ compiler (namely, Borland C++ Builder 4.0)?

8) Is there any conceivable circumstance where the original function
implementation could have been correct? Even if it's really a C++
class member function?
</OT>

9) Any other suggestions or nitpicks?
I hate multiple returns. I have used them in some realtime code, BUT I
still hate them. How about this :

char GetSortMethod( int sortMeth )
{
char ret_val;

ret_val = -1;

if( sortMeth == 0 )
ret_val = 'D';
if( sortMeth == 1 )
ret_val = 'A';

return(ret_val);
}
 
D

Dan Pop

In said:
(I did not write this code!)

const char *GetSortMethod( unsigned int sortMeth )
{
const char *sortingstring;
if( sortMeth == 0 ) {
sortingstring = "D";
}
if( sortMeth == 1 ) {
sortingstring = "A";
}
if( sortMeth == -1 ) {
sortingstring = "";
}
return( sortingstring );
}

1) Returning an automatic pointer to a string literal is a big
mistake, correct?

Wrong. It's perfectly OK, especially since it's a pointer to const char.
Returning a pointer to automatically allocated data is a big mistake.
Returning the value of automatically allocated data is fine.
2) If sortMeth isn't -1, 0, or 1, the value returned i
even less determinate than it was before...

If sortMeth isn't UINT_MAX, 0 or 1, the value returned is indeterminate.
3) sortMeth is *never* -1, since it's unsigned...

OTOH, sortMeth == -1 evaluates to true when sortMeth is UINT_MAX, because
-1 is converted to unsigned int as part of the == operator evaluation.
4) How about the following correction?

char GetSortMethod( int sortMeth )
{
return( !sortMeth ? 'D' : sortMeth == 1 ? 'A' : 0 );
}

5) No, just kidding! Seriously this time:

char GetSortMethod( int sortMeth )
{
if( sortMeth == 0 )
return 'D';
if( sortMeth == 1 )
return 'A';
return '\0';
}

Any *good* reason for not making GetSortMethod return int? Its return
value would be promoted to int in *any* context where it is actually
used, anyway.
6) The original function was used like this:

printf( "somevar='%s'\n", GetSortMethod(anum) ); /* int anum; */

Is this the best way to use the correct version?

char meth[2];
meth[0]=GetSortMethod( anum );
meth[1]='\0';

printf( "somevar='%s'\n", meth );

Obviously not!

printf("somevar='%c'\n", GetSortMethod(anum));

If the idea of printing the null character bothers you, you can do the
following:

char c = GetSortMethod(anum);
printf("somevar='%.1s'\n", &c);

Dan
 
C

Christopher Benson-Manica

James Beck said:
char ret_val;
ret_val = -1;

It's implementation-defined whether an unadorned char is signed or
unsigned, so this invokes implementation-defined behavior.

(Right?)
 
J

James Beck

It's implementation-defined whether an unadorned char is signed or
unsigned, so this invokes implementation-defined behavior.

(Right?)
Yep, I left out a comment that I had in my snippet at first....

/* assumming chars are signed */
 
E

Ed Morton

Joona said:
Christopher Benson-Manica <[email protected]> scribbled the following:


Right.

Well.... depends what you mean by "-1" :cool:. Try using this "main":

int main(void)
{
printf("%s\n",GetSortMethod(-1));
return 0;
}

and you'll see sortMeth == -1 evaluate to true within GetSortMeth().

Ed.
 
S

Severian

(I did not write this code!)

const char *GetSortMethod( unsigned int sortMeth )
{
const char *sortingstring;
if( sortMeth == 0 ) {
sortingstring = "D";
}
if( sortMeth == 1 ) {
sortingstring = "A";
}
if( sortMeth == -1 ) {
sortingstring = "";
}
return( sortingstring );
}

1) Returning an automatic pointer to a string literal is a big
mistake, correct?
2) If sortMeth isn't -1, 0, or 1, the value returned i
even less determinate than it was before...
3) sortMeth is *never* -1, since it's unsigned...
4) How about the following correction?

char GetSortMethod( int sortMeth )
{
return( !sortMeth ? 'D' : sortMeth == 1 ? 'A' : 0 );
}

5) No, just kidding! Seriously this time:

char GetSortMethod( int sortMeth )
{
if( sortMeth == 0 )
return 'D';
if( sortMeth == 1 )
return 'A';
return '\0';
}

6) The original function was used like this:

printf( "somevar='%s'\n", GetSortMethod(anum) ); /* int anum; */

Is this the best way to use the correct version?

char meth[2];
meth[0]=GetSortMethod( anum );
meth[1]='\0';

printf( "somevar='%s'\n", meth );

<OT>
7) Is the original function implementation any more viable when
compiled by a C++ compiler (namely, Borland C++ Builder 4.0)?

8) Is there any conceivable circumstance where the original function
implementation could have been correct? Even if it's really a C++
class member function?
</OT>

9) Any other suggestions or nitpicks?

Here's my suggestion. Simple, direct, returns a string where you want
a string.

const char * GetSortMethod(unsigned int meth)
{
switch (meth) {
case 0: return "D";
case 1: return "A";
default: return "";
}
}
 
D

Dan Pop

In said:
Got it.



If GetSortMethod returns '\0', the single quotes printed must not be
separated by whitespace.

Where did you get the idea that the null character counts as white space
from?

Dan
 
C

Christopher Benson-Manica

Dan Pop said:
Any *good* reason for not making GetSortMethod return int? Its return
value would be promoted to int in *any* context where it is actually
used, anyway.

It always returns a real character, right? Why not declare it such
that that fact is obvious?
char c = GetSortMethod(anum);
printf("somevar='%.1s'\n", &c);

Thank you - I should have seen that myself.
 
C

Christopher Benson-Manica

Dan Pop said:
Where did you get the idea that the null character counts as white space
from?

printf( "The null character is '%c'\n", '\0' );

prints ' ' with gcc for cygwin. I have no idea what the Standard says
about this situation.
 
P

pete

Christopher said:
(I did not write this code!)

const char *GetSortMethod( unsigned int sortMeth )
{
const char *sortingstring;
if( sortMeth == 0 ) {
sortingstring = "D";
}
if( sortMeth == 1 ) {
sortingstring = "A";
}
if( sortMeth == -1 ) {
sortingstring = "";
}
return( sortingstring );
}

1) Returning an automatic pointer to a string literal is a big
mistake, correct?

You can't return an automatic pointer, you can only return it's value.
If the value is the address of an object with static duration,
then you're OK.
 
P

Peter Nilsson

Christopher Benson-Manica said:
printf( "The null character is '%c'\n", '\0' );

prints ' ' with gcc for cygwin. I have no idea what the Standard says
about this situation.

Assuming stdout is a text stream at the time of execution...

5.2.2 Character display semantics
1 The active position is that location on a display device where the
next
character output by the fputc function would appear. The intent of
writing a
printing character (as defined by the isprint function) to a display
device
is to display a graphic representation of that character at the
active
position and then advance the active position to the next position
on the
current line.

If isprint(0) is 0, then there is no defined display semantics for the
null character on the given implementation.
 
Z

Zoran Cutura

James Beck said:
I hate multiple returns. I have used them in some realtime code, BUT I
still hate them. How about this :

If you hate multiple returns, why would you allow for 'em in realtime
code? What makes you think that RT-code would actually need 'em?
 
D

Dan Pop

In said:
printf( "The null character is '%c'\n", '\0' );

prints ' ' with gcc for cygwin. I have no idea what the Standard says
about this situation.

According to the C standard, isspace(0) returns 0 in the C locale.

It's not the job of the C standard to provide the full semantics of a
text terminal. The original purpose of the NUL character was to be used
as a padding character on terminals connected via a serial line, when the
previous character(s) contained a command which could not be *always*
completed until the next character arrived, so the next character(s) were
padding characters. If the command was executed fast enough (e.g. the
printing head didn't have to be moved from one end of the paper to the
other when a CR character was received) the terminal could actually
receive the padding character(s) in time. If this happened, it was
supposed to ignore them.

The chapter and verse from the ISO 646 standard:

A control character used to accomplish media-fill or time-fill. Null
characters may be inserted into or removed from a stream of data
without affecting the information content of that stream. But then
^^^^^^^
the addition or removal of these characters may affect the information
layout and/or the control of equipment.

So, the following program is supposed to display "abc".

fangorn:~/tmp 184> cat test.c
#include <stdio.h>

int main()
{
printf("a%cb%cc\n", 0, 0);
return 0;
}
fangorn:~/tmp 185> gcc test.c
fangorn:~/tmp 186> ./a.out
abc

The cygwin people got their terminal emulation wrong.

Dan
 
J

James Beck

If you hate multiple returns, why would you allow for 'em in realtime
code? What makes you think that RT-code would actually need 'em?

Why not? I can say that I've never used a goto ;)

Jim
 
R

Richard Heathfield

James said:
Why not? I can say that I've never used a goto ;)

If you don't like multiple returns, you don't have to use them.
If you don't like goto, you don't have to use it.
If you don't like either of them, you don't have to use either of them.
 
J

James Beck

If you don't like multiple returns, you don't have to use them.
If you don't like goto, you don't have to use it.
If you don't like either of them, you don't have to use either of them.
Thanks Captain Obvious........

on to your next task.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,755
Messages
2,569,537
Members
45,023
Latest member
websitedesig25

Latest Threads

Top