Re: please verify the code
 

Roman Mashak wrote:
> 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: mrv@tusur.ru
>
>


--
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

please verify the code
 

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: mrv@tusur.ru

Re: please verify the code
 

On Fri, 16 Dec 2005 21:27:15 -0800, Thomas Matthews
<Thomas_Hates_Spam@cox.network> wrote in comp.lang.c:

> Roman Mashak wrote:

[large snip]

> > return 0;
> 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.

--
Jack Klein
Home: http://JK-Technology.Com
FAQs for
comp.lang.c http://www.eskimo.com/~scs/C-faq/top.html
comp.lang.c++ http://www.parashift.com/c++-faq-lite/
alt.comp.lang.learn.c-c++
http://www.contrib.andrew.cmu.edu/~a...FAQ-acllc.html

Re: please verify the code
 

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: mrv@tusur.ru

Re: please verify the code
 

Roman Mashak wrote:
>
> 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

Re: please verify the code
 

pete wrote:
>
> Roman Mashak wrote:
> >
> > 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

Re: please verify the code
 

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 */


--
pete

Re: please verify the code
 

Jack Klein wrote:

> On Fri, 16 Dec 2005 21:27:15 -0800, Thomas Matthews
> <Thomas_Hates_Spam@cox.network> wrote in comp.lang.c:
>
>
>>Roman Mashak wrote:
>
>
> [large snip]
>
>
>>> return 0;
>>
>>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>.

Re: please verify the code
 

Roman Mashak wrote:
> 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.

--
Paul Hsieh
http://www.pobox.com/~qed/
http://bstring.sf.net/

Re: please verify the code
 

websnarf@gmail.com wrote:
> Roman Mashak wrote:
>
>>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?

--
Joe Wright
"Everything should be made as simple as possible, but not simpler."
--- Albert Einstein ---