please verify the code

[Thomas Matthews]

Hello, All!

I wrote function parsing the fully-quilified domain name and extracting
'host' and 'domain' parts seperately. The important thing for me is to keep
original string (pointed by 'fqdn' in function). Is my way of coding
correct, is there a way to optimize function and make it not so clumsy (I
like the code produced by K&R in their famous book: brief, clear,
comprehensible, nothing superfluous ).

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

#define BUFLEN 1024

static int
parse_fqdn(char *fqdn, char *host, char *domain)
{
char p[BUFLEN], *s;

Think about:
1. 1024 chars is a lot to place on the local variable area.
2. One variable declaration per line.
3. Is the variable 'p' necessary?
4. If a parameter points to a constant string or
the function will not modify the string, consider
declaring the parameter as a pointer to a const char.

strcpy(p, fqdn);

Think about:
1. What happens when the length of fqdn is greater than BUFLEN?
2. What happens when fqdn is NULL?

if ( (s = strstr(p, ".")) ) {

Think about:
1. To prevent typeos and logic errors, compare the result
of a function to a variable or constant.

*s = '\0';
strcpy(host, p);
strcpy(domain, ++s);
return 0;
}
else
return -1;
}

int main(void)
{
char dom[] = "www.my.example.dom.ain";
char h[BUFLEN], d[BUFLEN];

if ( parse_fqdn(dom, h, d) == 0 )
printf("fqdn='%s'\nhostname='%s', domain name='%s'\n", dom, h, d);
return 0;

Although "return 0;" is valid, consider using
EXIT_SUCCESS or EXIT_FAILURE. These two named constants
make the code more readable.

}

Thanks in advance

With best regards, Roman Mashak. E-mail: (e-mail address removed)

--
Thomas Matthews

C++ newsgroup welcome message:
http://www.slack.net/~shiva/welcome.txt
C++ Faq: http://www.parashift.com/c++-faq-lite
C Faq: http://www.eskimo.com/~scs/c-faq/top.html
alt.comp.lang.learn.c-c++ faq:
http://www.comeaucomputing.com/learn/faq/
Other sites:
http://www.josuttis.com -- C++ STL Library book
http://www.sgi.com/tech/stl -- Standard Template Library

 

[Roman Mashak]

Hello, All!

I wrote function parsing the fully-quilified domain name and extracting
'host' and 'domain' parts seperately. The important thing for me is to keep
original string (pointed by 'fqdn' in function). Is my way of coding
correct, is there a way to optimize function and make it not so clumsy (I
like the code produced by K&R in their famous book: brief, clear,
comprehensible, nothing superfluous ).

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

#define BUFLEN 1024

static int
parse_fqdn(char *fqdn, char *host, char *domain)
{
char p[BUFLEN], *s;

strcpy(p, fqdn);
if ( (s = strstr(p, ".")) ) {
*s = '\0';
strcpy(host, p);
strcpy(domain, ++s);
return 0;
}
else
return -1;
}

int main(void)
{
char dom[] = "www.my.example.dom.ain";
char h[BUFLEN], d[BUFLEN];

if ( parse_fqdn(dom, h, d) == 0 )
printf("fqdn='%s'\nhostname='%s', domain name='%s'\n", dom, h, d);
return 0;
}

Thanks in advance

With best regards, Roman Mashak. E-mail: (e-mail address removed)

 

[Jack Klein]

Roman Mashak wrote:

[large snip]

Although "return 0;" is valid, consider using
EXIT_SUCCESS or EXIT_FAILURE. These two named constants
make the code more readable.

It is pedantic beyond reason to claim that "return 0;" is less
readable than "return EXIT_SUCCESS;" to anyone remotely qualified to
read C code.

I _never_ use EXIT_SUCCESS in a program unless I also use
EXIT_FAILURE, which the OP's program does not.

 

[Roman Mashak]

Hello, Thomas!
You wrote on Fri, 16 Dec 2005 21:27:15 -0800:

??>> parse_fqdn(char *fqdn, char *host, char *domain)
??>> {
??>> char p[BUFLEN], *s;
TM> Think about:
TM> 1. 1024 chars is a lot to place on the local variable area.
[OT]
AFAIR the maximum length of fully-quilifed is much less than 1024
[/OT]
TM> 2. One variable declaration per line.
It's simply coding style, isn't it?
TM> 3. Is the variable 'p' necessary?
I use it to keep the backup copy of original string (fqdn), is there a way
to preserve it not defining extra variable?
TM> 4. If a parameter points to a constant string or
TM> the function will not modify the string, consider
TM> declaring the parameter as a pointer to a const char.
correct point, thanks
??>> strcpy(p, fqdn);
TM> Think about:
TM> 1. What happens when the length of fqdn is greater than BUFLEN?
TM> 2. What happens when fqdn is NULL?
correct, I have to check these conditions
??>> if ( (s = strstr(p, ".")) ) {
TM> Think about:
TM> 1. To prevent typeos and logic errors, compare the result
TM> of a function to a variable or constant.
What do you mean here, please clarify.


With best regards, Roman Mashak. E-mail: (e-mail address removed)

 

[pete]

Hello, All!

I wrote function parsing the fully-quilified domain name and extracting
'host' and 'domain' parts seperately. The important thing for me is to keep
original string (pointed by 'fqdn' in function). Is my way of coding
correct, is there a way to optimize function and make it not so clumsy (I
like the code produced by K&R in their famous book: brief, clear,
comprehensible, nothing superfluous ).

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

#define BUFLEN 1024

static int
parse_fqdn(char *fqdn, char *host, char *domain)
{
char p[BUFLEN], *s;

strcpy(p, fqdn);
if ( (s = strstr(p, ".")) ) {
*s = '\0';
strcpy(host, p);
strcpy(domain, ++s);
return 0;
}
else
return -1;
}

int main(void)
{
char dom[] = "www.my.example.dom.ain";
char h[BUFLEN], d[BUFLEN];

if ( parse_fqdn(dom, h, d) == 0 )
printf("fqdn='%s'\nhostname='%s', domain name='%s'\n", dom, h, d);
return 0;
}

Thanks in advance

My preferences are:
1 To pass in the p buffer as an argument.
That gives you more control and flexibility.
Then p can be either static or automatic
or point to allocated memory.
2 To use the same convention as strchr for returning
success or failure, since the return value of strchr
is what determines that, for this function.
3 Not to have side effects in arguments,
as in strcpy(domain, s++)
4 To use the const qualifier for char *fqdn.

/* BEGIN new.c */

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

#define BUFLEN 1024

char *
parse_fqdn(const char *fqdn, char *p, char *host, char *domain)
{
char *s;

strcpy(p, fqdn);
if ( (s = strstr(p, ".")) ) {
*s++ = '\0';
strcpy(host, p);
strcpy(domain, s);
}
return s;
}

int main(void)
{
char dom[] = "www.my.example.dom.ain";
char p[BUFLEN], h[BUFLEN], d[BUFLEN];

if ( parse_fqdn(dom, p, h, d) != NULL ) {
printf("fqdn='%s'\nhostname='%s', domain name='%s'\n"
, dom, h, d);
}
return 0;
}

/* END new.c */

 

[pete]

Hello, All!

I wrote function parsing the fully-quilified domain name and extracting
'host' and 'domain' parts seperately. The important thing for me is to keep
original string (pointed by 'fqdn' in function). Is my way of coding
correct, is there a way to optimize function and make it not so clumsy (I
like the code produced by K&R in their famous book: brief, clear,
comprehensible, nothing superfluous ).

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

#define BUFLEN 1024

static int
parse_fqdn(char *fqdn, char *host, char *domain)
{
char p[BUFLEN], *s;

strcpy(p, fqdn);
if ( (s = strstr(p, ".")) ) {
*s = '\0';
strcpy(host, p);
strcpy(domain, ++s);
return 0;
}
else
return -1;
}

int main(void)
{
char dom[] = "www.my.example.dom.ain";
char h[BUFLEN], d[BUFLEN];

if ( parse_fqdn(dom, h, d) == 0 )
printf("fqdn='%s'\nhostname='%s', domain name='%s'\n", dom, h, d);
return 0;
}

Thanks in advance

My preferences are:
1 To pass in the p buffer as an argument.
That gives you more control and flexibility.
Then p can be either static or automatic
or point to allocated memory.
2 To use the same convention as strchr for returning
success or failure, since the return value of strchr
is what determines that, for this function.
3 Not to have side effects in arguments,
as in strcpy(domain, s++)
4 To use the const qualifier for char *fqdn.

/* BEGIN new.c */

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

#define BUFLEN 1024

char *
parse_fqdn(const char *fqdn, char *p, char *host, char *domain)
{
char *s;

strcpy(p, fqdn);
if ( (s = strstr(p, ".")) ) {

/* I also meant to replace strstr with strchr */

if ( (s = strchr(p, '.')) ) {

*s++ = '\0';
strcpy(host, p);
strcpy(domain, s);
}
return s;
}

int main(void)
{
char dom[] = "www.my.example.dom.ain";
char p[BUFLEN], h[BUFLEN], d[BUFLEN];

if ( parse_fqdn(dom, p, h, d) != NULL ) {
printf("fqdn='%s'\nhostname='%s', domain name='%s'\n"
, dom, h, d);
}
return 0;
}

/* END new.c */

 

[pete]

pete wrote:

My preferences are:
1 To pass in the p buffer as an argument.
That gives you more control and flexibility.
Then p can be either static or automatic
or point to allocated memory.

/* BEGIN new.c */

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

char *
parse_fqdn(const char *fqdn, char *p, char *host, char *domain)
{
char *s;

strcpy(p, fqdn);
if ( (s = strchr(p, '.')) ) {
*s++ = '\0';
strcpy(host, p);
strcpy(domain, s);
}
return s;
}

int main(void)
{
char dom[] = "www.my.example.dom.ain";
char *p, *h, *d;

p = malloc(strlen(dom) + 1);
h = malloc(strlen(dom));
d = malloc(strlen(dom));
if (p != NULL && h != NULL && d != NULL) {
if ( parse_fqdn(dom, p, h, d) != NULL ) {
printf("fqdn='%s'\nhostname='%s', domain name='%s'\n"
, dom, h, d);
}
} else {
puts("malloc problem");
}
free(d);
free(h);
free(p);
return 0;
}

/* END new.c */

 

[Bill Gutz]

Roman Mashak wrote:

[large snip]

Although "return 0;" is valid, consider using
EXIT_SUCCESS or EXIT_FAILURE. These two named constants
make the code more readable.

It is pedantic beyond reason to claim that "return 0;" is less
readable than "return EXIT_SUCCESS;" to anyone remotely qualified to
read C code.

I _never_ use EXIT_SUCCESS in a program unless I also use
EXIT_FAILURE, which the OP's program does not.

Same here. If you use EXIT_FAILURE, or EXIT_SUCCESS, then you have to
include <stdlib.h>.

 

[websnarf]

I wrote function parsing the fully-quilified domain name and extracting
'host' and 'domain' parts seperately.

I am highly skeptical that the *functionality* of your code is correct.
I usually take URLs to be of the form:
(<subdomainname>.)*<host-domainname>.<top-level-domainname> where
certain domain names have exceptional rules (co.uk, for example).
Ripping off the top most part usually just gets your some
sub-assignment by the host themselves.

[...] The important thing for me is to keep
original string (pointed by 'fqdn' in function). Is my way of coding
correct, is there a way to optimize function and make it not so clumsy (I
like the code produced by K&R in their famous book: brief, clear,
comprehensible, nothing superfluous ).

This newsgroup is poor place to come for "optimization" help, unless
you mean making it somehow look like K&R-style syntax or something like
that. But I happen to see your post so ...

First of all, why do you do the extra copying step through p? Its
unnecessary! Presumably, you have two destination buffers ready to
receive data, just do your final manipulations in there:

----------------------------------------

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

static int
parse_fqdn(char *fqdn, char *host, char *domain) {
char *s = strstr (fqdn, ".");
ptrdiff_t len = s - fqdn;

if (NULL == s) return -1;
memcpy (host, fqdn, len);
host[len] = '\0';
strcpy (domain, s + 1);
return 0;
}

#define BUFLEN 1024

int main(void) {
char dom[] = "www.my.example.dom.ain";
static char h[BUFLEN], d[BUFLEN];

if ( parse_fqdn(dom, h, d) == 0 )
printf("fqdn='%s'\nhostname='%s', domain name='%s'\n", dom, h,
d);
return EXIT_SUCCESS;
}

----------------------------------------

Ok, notice how I moved the BUFLEN definition below the parse_fqdn
definition? In this way you can make somewhat more enlightened
decisions about the target buffer length in the future, if necessary.
For example, in your main you could easily do this:

static char h[sizeof (dom)], d[sizeof (dom)];

and avoid the use of predefined fixed size buffers altogether. In more
dynamic situations, you would malloc char*'s with the strlen() of the
input strings or something to that effect.

Of course when you get tired of buffer overflows, performance problems,
and other buffer management issues, you can just use "The Better String
Library":

----------------------------------------

#include <stdlib.h>
#include <stdio.h>
#include "bstrlib.h"

int main(void) {
struct tagbstring dom = bsStatic ("www.my.example.dom.ain");
int i = bstrchr (&dom, '.'); /* I think bstrrchr is what you really
want here */
if (BSTR_ERR != i) {
bstring h = blk2bstr (dom.data, i);
bstring d = blk2bstr (dom.data + i + 1, dom.slen - (i + 1));
printf("fqdn='%s'\nhostname='%s', domain name='%s'\n",
dom.data,
bdatae (h, "<Out of memory>"),
bdatae (d, "<Out of memory>"));
bdestroy (h);
bdestroy (d);
}
return EXIT_SUCCESS;
}

 

[Joe Wright]

I wrote function parsing the fully-quilified domain name and extracting
'host' and 'domain' parts seperately.

I am highly skeptical that the *functionality* of your code is correct.
I usually take URLs to be of the form:
(<subdomainname>.)*<host-domainname>.<top-level-domainname> where
certain domain names have exceptional rules (co.uk, for example).
Ripping off the top most part usually just gets your some
sub-assignment by the host themselves.

[...] The important thing for me is to keep
original string (pointed by 'fqdn' in function). Is my way of coding
correct, is there a way to optimize function and make it not so clumsy (I
like the code produced by K&R in their famous book: brief, clear,
comprehensible, nothing superfluous ).

This newsgroup is poor place to come for "optimization" help, unless
you mean making it somehow look like K&R-style syntax or something like
that. But I happen to see your post so ...

First of all, why do you do the extra copying step through p? Its
unnecessary! Presumably, you have two destination buffers ready to
receive data, just do your final manipulations in there:

----------------------------------------

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

static int
parse_fqdn(char *fqdn, char *host, char *domain) {
char *s = strstr (fqdn, ".");
ptrdiff_t len = s - fqdn;

if (NULL == s) return -1;
memcpy (host, fqdn, len);
host[len] = '\0';
strcpy (domain, s + 1);
return 0;
}

#define BUFLEN 1024

int main(void) {
char dom[] = "www.my.example.dom.ain";
static char h[BUFLEN], d[BUFLEN];

if ( parse_fqdn(dom, h, d) == 0 )
printf("fqdn='%s'\nhostname='%s', domain name='%s'\n", dom, h,
d);
return EXIT_SUCCESS;
}

----------------------------------------

Ok, notice how I moved the BUFLEN definition below the parse_fqdn
definition? In this way you can make somewhat more enlightened
decisions about the target buffer length in the future, if necessary.
For example, in your main you could easily do this:

static char h[sizeof (dom)], d[sizeof (dom)];

and avoid the use of predefined fixed size buffers altogether. In more
dynamic situations, you would malloc char*'s with the strlen() of the
input strings or something to that effect.

Of course when you get tired of buffer overflows, performance problems,
and other buffer management issues, you can just use "The Better String
Library":

----------------------------------------

#include <stdlib.h>
#include <stdio.h>
#include "bstrlib.h"

int main(void) {
struct tagbstring dom = bsStatic ("www.my.example.dom.ain");
int i = bstrchr (&dom, '.'); /* I think bstrrchr is what you really
want here */
if (BSTR_ERR != i) {
bstring h = blk2bstr (dom.data, i);
bstring d = blk2bstr (dom.data + i + 1, dom.slen - (i + 1));
printf("fqdn='%s'\nhostname='%s', domain name='%s'\n",
dom.data,
bdatae (h, "<Out of memory>"),
bdatae (d, "<Out of memory>"));
bdestroy (h);
bdestroy (d);
}
return EXIT_SUCCESS;
}

----------------------------------------

Notice how there is no "#include <string.h>" here? As a good rule of
thumb -- there is a good chance you are introducing performance
problems if you have included that file.

Ok, I'll bite. How does "#include <string.h>" affect the performance of
anything that doesn't use it?

 

[Default User]

(e-mail address removed) wrote:

[big snip]

Ok, I'll bite. How does "#include <string.h>" affect the performance
of anything that doesn't use it?

Is there a reason you felt the need to quote the entire long post just
to add one little comment?




Brian

 

[Joe Wright]

Joe Wright wrote:

(e-mail address removed) wrote:

[big snip]

Ok, I'll bite. How does "#include <string.h>" affect the performance
of anything that doesn't use it?

Is there a reason you felt the need to quote the entire long post just
to add one little comment?

Just trying to include all context. Sorry if you were bothered.
Is there a reason you felt the need to chastise me for not snipping and
yet offer no other response to my post? Do you know of any performance
hit in including an unused header?

 

[Keith Thompson]

(e-mail address removed) wrote: [big snip]

Notice how there is no "#include <string.h>" here? As a good rule of
thumb -- there is a good chance you are introducing performance
problems if you have included that file.

Ok, I'll bite. How does "#include <string.h>" affect the performance of
anything that doesn't use it?

Obviously #include'ing a header that's not used isn't going to have
any effect on run-time performance. (I might be able to contrive a
case where it could, but I'm not going to try.)

I think websnarf's point was that a "#include <string.h>" is almost
always an indication that some of the functions declared in that
header are actually being used, and that those functions are less
efficient than his own string functions. It's not a direct
implication, but it's a reasonable inference. (No comment on whether
he's right about that.)

 

[Joe Wright]

(e-mail address removed) wrote:

[big snip]

Ok, I'll bite. How does "#include <string.h>" affect the performance of
anything that doesn't use it?

Obviously #include'ing a header that's not used isn't going to have
any effect on run-time performance. (I might be able to contrive a
case where it could, but I'm not going to try.)

I think websnarf's point was that a "#include <string.h>" is almost
always an indication that some of the functions declared in that
header are actually being used, and that those functions are less
efficient than his own string functions. It's not a direct
implication, but it's a reasonable inference. (No comment on whether
he's right about that.)

Thanks for the 'clarification' but he didn't say that. Paul Hsieh said
there was a good chance of introducing performance problems by including
the header. Nonsense. There is NO chance that you or I know of.

 

[Keith Thompson]

(e-mail address removed) wrote: [big snip]
Notice how there is no "#include <string.h>" here? As a good rule of
thumb -- there is a good chance you are introducing performance
problems if you have included that file.

Ok, I'll bite. How does "#include <string.h>" affect the performance of
anything that doesn't use it?

Obviously #include'ing a header that's not used isn't going to have
any effect on run-time performance. (I might be able to contrive a
case where it could, but I'm not going to try.)
I think websnarf's point was that a "#include <string.h>" is almost
always an indication that some of the functions declared in that
header are actually being used, and that those functions are less
efficient than his own string functions. It's not a direct
implication, but it's a reasonable inference. (No comment on whether
he's right about that.)

Thanks for the 'clarification' but he didn't say that. Paul Hsieh said
there was a good chance of introducing performance problems by
including the header. Nonsense. There is NO chance that you or I know
of.

Of course it doesn't, and you, I, and websnarf are all perfectly well
aware of that. His statement was indirect, but (arguably) perfectly
correct. He wrote:

... there is a good chance you are introducing performance
problems if you have included that file.

He wrote "if you have included that file", *not* "by including that
file". If you've included <string.h>, there is a good chance that
you've done so for the purpose of calling functions declared in that
header, and caling those functions can (arguably) introduce
performance problems.

Even a pedantically literal reading of what he wrote doesn't imply
that the #include directive itself affects performance.

 

[websnarf]

Ok, I'll bite. How does "#include <string.h>" affect the performance of
anything that doesn't use it?

Its a rule of thumb -- and it seems I got it wrong. You need string.h
for things like memcpy(), which of course it not a performance problem.
Its really the use of str* functions that tells you you have a
potential performance problem. So I'll go back to my old heuristic
which says if you call strlen(), then you have thrown performance out
the window.

 

[Default User]

Default User wrote:

Just trying to include all context. Sorry if you were bothered.

All the context was not needed. What I left in was ample. Under your
theory, no one should ever snip anything.

Is there a reason you felt the need to chastise me for not snipping

Yes, it's rude not to do so.

and
yet offer no other response to my post?

That's all I had to say.




Brian