Thread: [Libclc-developers] Some more comments to the current CVS tree
Status: Planning
Brought to you by:
augestad
Menu
▾
▴
libclc-developers
|
From: Jan E. <je...@li...> - 2003-04-02 16:29:16
|
Hi there,
whilst looking through the current CVS snapshots, I stumbled over these issues:
doc/string/clc_stralloc.txt:
char *s = clc_stralloc("foo", "bar", (char *)0);
Can't we assume that NULL is (something casted) 0? Yet,
NULL is defined as (void *)0, but that is not a big difference
to (char *)0. Even (int *)0 would work, wouldn't it?
src/clc_stralloc.c:
va_start() is used two times. IIRC, such use is not recommended,
for whatever reason. IIRC.
They propose va_copy() instead.
src/clc_strcasecmp.c:
for (;; s1++, s2++)
Euh, maybe written as while(1) { ...; ++s1; ++s2 } ?
src/clc_stpcpy.c:
It is using its own copy mechanism. Since strcpy() is defined in ISO
9899 (is that C 89 or C 99 ?), oh well. If strcpy() is C89, wouldn't
it be wiser to stick to that?
src/clc_strdup.c:
I am a great destroyer of readability >:-) so I propose
if((size = strlen(s) + 1) == 0) { p = NULL; }
instead of the two lines
size = strlen(s) + 1
if(size == 0) { p = NULL; }
src/clc_strlcpy.c:
You cannot get rid of me.
- while(*src)
- ++src
+ while(*src++);
src/clc_strltrim.c:
char *cptr;
cptr = s;
while(... && isspace((unsigned char)*cptr))
What if you would make cptr unsigned at the beginning?
unsigned char *cptr;
assert...
cptr=s;
while(...&&isspace(*cptr))
doc/string/clc_strrev.txt
small indent problem if you care ;)
doc/string/*
General indent question. Some files have 4 some have 2 leading.
=)
- Jan Engelhardt
|
|
From: regis <re...@in...> - 2003-04-02 16:45:58
|
Jan Engelhardt wrote:
>
> Hi there,
>
> whilst looking through the current CVS snapshots, I stumbled over these issues:
>
> doc/string/clc_stralloc.txt:
> char *s = clc_stralloc("foo", "bar", (char *)0);
>
> Can't we assume that NULL is (something casted) 0? Yet,
> NULL is defined as (void *)0, but that is not a big difference
> to (char *)0. Even (int *)0 would work, wouldn't it?
No.
(char *)0 is an argument that is part part of the ellipsis
in the variadic function. So, there is no type conversion:
an arg of the exact type muist be pass, therefore the cast.
If a bare 0 is passed, then an int is passed, and
1) its sizeof may differ from the expected pointer
2) its zero-representation may not be the repr. of
the null pointer for this type. An implmentation may
decide e.g. that the hex value 0xDEADBEEF is the repr.
of a null pointer.
And the repr. of two null pointers for two different types
may differ.
|
|
From: regis <re...@in...> - 2003-04-02 16:54:27
|
Jan Engelhardt wrote:
> src/clc_strcasecmp.c:
> for (;; s1++, s2++)
>
> Euh, maybe written as while(1) { ...; ++s1; ++s2 } ?
while (1) is idiomatically a nonsense.
while(condition) is for conditionnal continuation of loops.
if there is no condition, there while should not be used.
The idiomatic construct for non-conditional loop is for(;;)
with empty continuation condition.
> src/clc_strlcpy.c:
> You cannot get rid of me.
> - while(*src)
> - ++src
> + while(*src++);
No gain except obfuscation of code.
Ironically, you propose to do here
the reverse of your proposal for src/clc_strcasecmp.c:
above...
|
|
From: Hallvard B F. <h.b...@us...> - 2003-04-02 17:15:38
|
Jan Engelhardt writes:
> whilst looking through the current CVS snapshots, I stumbled over
> these issues:
Now that it has been posted to c.l.c, it might be better to discuss it
there. maybe that would liven c.l.c a bit. However,
> doc/string/clc_stralloc.txt:
> char *s = clc_stralloc("foo", "bar", (char *)0);
>
> Can't we assume that NULL is (something casted) 0?
No. See Regis' reply, or the documentation.
> src/clc_stralloc.c:
> va_start() is used two times. IIRC, such use is not recommended,
> for whatever reason. IIRC.
> They propose va_copy() instead.
Can't. va_copy() is a C99 macro. It's not defined in C89.
> src/clc_strcasecmp.c:
> for (;; s1++, s2++)
>
> Euh, maybe written as while(1) { ...; ++s1; ++s2 } ?
Well, I like it the way it is. If I'm voted down, I'll change it.
> src/clc_stpcpy.c:
> It is using its own copy mechanism. Since strcpy() is defined in ISO
> 9899 (is that C 89 or C 99 ?), oh well. If strcpy() is C89, wouldn't
> it be wiser to stick to that?
I'm not quite sure what you mean - is that a vote against the function,
or the implementation? The function exists because its result can be
faster to use than strcat(), which must traverse the first argument.
The implementation does its own copying rather than
strcpy(s1, s2);
return s1 + strlen(s1);
beacuse the former hopefully is quicker.
> src/clc_strdup.c:
> I am a great destroyer of readability >:-)
So you are:-)
> so I propose
> if((size = strlen(s) + 1) == 0) { p = NULL; }
> instead of the two lines
> size = strlen(s) + 1
> if(size == 0) { p = NULL; }
Why?
> src/clc_strlcpy.c:
> You cannot get rid of me.
> - while(*src)
> - ++src
> + while(*src++);
That doesn't help, then we must do --src; afterwards.
> src/clc_strltrim.c:
> (...)
> while(... && isspace((unsigned char)*cptr))
> What if you would make cptr unsigned at the beginning?
Well, it's one cast in either case. I have no preference.
However, there is a difference between the two in case char is signed
and either sign/magnitude or one's complement. I have no idea which is
right in that case. Looks like c.l.c material.
> doc/string/clc_strrev.txt
> small indent problem if you care ;)
>
> doc/string/*
> General indent question. Some files have 4 some have 2 leading.
8 and 4, that is. Fixed.
--
Hallvard
|
|
From: Hallvard B F. <h.b...@us...> - 2003-04-02 17:45:12
|
I wrote:
>> doc/string/clc_stralloc.txt:
>> char *s =3D clc_stralloc("foo", "bar", (char *)0);
>>=20
>> Can't we assume that NULL is (something casted) 0?
>=20
> No. See Regis' reply, or the documentation.
Um, that is, see the other version of the documentation.
Bj=F8rn, you have put up an outdated version of the documentation,
this one: Subject: [Libclc-developers] String functions so far
instead of: Subject: [Libclc-developers] String functions so far (2)
Shall I replace it, or is this something I should leave to you
or the documentation manager?
--=20
Hallvard
|
|
From: <bo...@me...> - 2003-04-02 18:17:32
|
Hallvard B Furuseth wrote: > > Bjørn, you have put up an outdated version of the documentation, Man, I'm sloppy these days. :-( > this one: Subject: [Libclc-developers] String functions so far > instead of: Subject: [Libclc-developers] String functions so far (2) > Shall I replace it, or is this something I should leave to you > or the documentation manager? How about updating the CVS with the proper version and then notify Bertrand? I guess he's reading this anyhow. ;-) -- boa libclc home: http://libclc.sourceforge.net |
|
From: Hallvard B F. <h.b...@us...> - 2003-04-02 18:36:58
|
Bj=F8rn Augestad writes: >Hallvard B Furuseth wrote: >>=20 >> Bj=F8rn, you have put up an outdated version of the documentation, > (...) > How about updating the CVS with the proper version and then notify=20 > Bertrand? Done. --=20 Hallvard |
|
From: Jan E. <je...@li...> - 2003-04-02 18:16:52
|
>>> src/clc_stralloc.c: >>> va_start() is used two times. IIRC, such use is not recommended, >>> for whatever reason. IIRC. >>> They propose va_copy() instead. >> >> Can't. va_copy() is a C99 macro. It's not defined in C89. > >If you remember where you have this recommendation from, can you bring >it up on c.l.c, now that the string functions have been posted there? >Hm. Or if you don't remember too, for that matter. Hm. Don't seem to find it. Whatever. va_copy(a,b) is [usually] only defined as a = b, at least in GCC, what is what I was talking about. - Jan Engelhardt |
|
From: Jan E. <je...@li...> - 2003-04-03 13:53:52
|
>> src/clc_stralloc.c:
>> va_start() is used two times. IIRC, such use is not recommended,
>> for whatever reason. IIRC.
>> They propose va_copy() instead.
>
>Can't. va_copy() is a C99 macro. It's not defined in C89.
Still, side effects may occur. (Still suggesting ap2=ap1 before doing
any va_start, need we?)
>> src/clc_stpcpy.c:
>> It is using its own copy mechanism. Since strcpy() is defined in ISO
>> 9899 (is that C 89 or C 99 ?), oh well. If strcpy() is C89, wouldn't
>> it be wiser to stick to that?
>
>I'm not quite sure what you mean - is that a vote against the function,
>or the implementation? The function exists because its result can be
I mean, if strcpy() is in the C89 standard, why doesnot stpcpy use
strcpy?
>> src/clc_strdup.c:
>> I am a great destroyer of readability >:-)
>So you are:-)
As I said!?
>> so I propose
>> if((size = strlen(s) + 1) == 0) { p = NULL; }
>> instead of the two lines
>> size = strlen(s) + 1
>> if(size == 0) { p = NULL; }
>Why?
"To destroy readability"
>> src/clc_strltrim.c:
>> (...)
>> while(... && isspace((unsigned char)*cptr))
>> What if you would make cptr unsigned at the beginning?
>
>Well, it's one cast in either case. I have no preference.
>However, there is a difference between the two in case char is signed
>and either sign/magnitude or one's complement. I have no idea which is
>right in that case. Looks like c.l.c material.
I think it is just another stylie thing
>> doc/string/*
>> General indent question. Some files have 4 some have 2 leading.
(Meant some files have 8 for paragraphs (4 for subheadings), whereas
others had 4 for paragraphs and 2 for subheadings)
- Jan Engelhardt
|
|
From: Hallvard B F. <h.b...@us...> - 2003-04-03 14:18:53
|
Jan Engelhardt writes: >>> src/clc_stralloc.c: >>> va_start() is used two times. IIRC, such use is not recommended, >>> for whatever reason. IIRC. >>> They propose va_copy() instead. >> >>Can't. va_copy() is a C99 macro. It's not defined in C89. > > Still, side effects may occur. (Still suggesting ap2=ap1 before doing > any va_start, need we?) That shouldn't hurt. But let's take it to c.l.c. > I mean, if strcpy() is in the C89 standard, why doesnot stpcpy use > strcpy? Because it's hopefully faster not to. It avoids a strlen() call. >>> doc/string/* >>> General indent question. Some files have 4 some have 2 leading. > > (Meant some files have 8 for paragraphs (4 for subheadings), whereas > others had 4 for paragraphs and 2 for subheadings) Oops, I didn't notice you were talking about doc not src. Anyway, it's fixed now. -- Hallvard |
|
From: Jan E. <je...@li...> - 2003-04-03 13:55:20
|
>> doc/string/clc_stralloc.txt:
>> char *s = clc_stralloc("foo", "bar", (char *)0);
>>
>> Can't we assume that NULL is (something casted) 0? Yet,
>> NULL is defined as (void *)0, but that is not a big difference
>> to (char *)0. Even (int *)0 would work, wouldn't it?
>
>No.
>
>(char *)0 is an argument that is part part of the ellipsis
>in the variadic function. So, there is no type conversion:
>an arg of the exact type muist be pass, therefore the cast.
>If a bare 0 is passed, then an int is passed, and
umm I proposed (void *)0, which is not a bare 0, but may
not necessarily be (char*)0?
- Jan Engelhardt
|
|
From: Hallvard B F. <h.b...@us...> - 2003-04-03 14:15:17
|
Jan Engelhardt writes:
>>> doc/string/clc_stralloc.txt:
>>> char *s = clc_stralloc("foo", "bar", (char *)0);
>>>
>>> Can't we assume that NULL is (something casted) 0? (...)
>>
>> No. (...)
>
> umm I proposed (void *)0, which is not a bare 0, but may
> not necessarily be (char*)0?
You proposed NULL. That may be #defined as 0. That is null pointer
constant (see ANSI 3.2.2.3 Pointers), like the Standard requires of NULL
(in 4.1.5 Common definitions <stddef.h>).
--
Hallvard
|
|
From: Jan E. <je...@li...> - 2003-04-03 16:17:40
|
>>>> doc/string/clc_stralloc.txt:
>>>> char *s = clc_stralloc("foo", "bar", (char *)0);
>>>>
>>>> Can't we assume that NULL is (something casted) 0? (...)
>>>
>>> No. (...)
>>
>> umm I proposed (void *)0, which is not a bare 0, but may
>> not necessarily be (char*)0?
>
>You proposed NULL. That may be #defined as 0. That is null pointer
>constant (see ANSI 3.2.2.3 Pointers), like the Standard requires of NULL
>(in 4.1.5 Common definitions <stddef.h>).
:(
But stdio.h (at least glib/gcc) has it as (void *)0 defined.
Maybe we gonna introduce some CLC_NULL :D
- Jan Engelhardt
|
|
From: Hallvard B F. <h.b...@us...> - 2003-04-03 19:14:42
|
Jan Engelhardt writes: > But stdio.h (at least glib/gcc) has it as (void *)0 defined. On Solaris it's defined as 0 or 0L, depending on the architecture. So it has the decency to #define NULL as an integer with the same representation as (void *)0, but it didn't have to do that. -- Hallvard |
|
From: Jan E. <je...@li...> - 2003-04-03 19:34:34
|
>> But stdio.h (at least glib/gcc) has it as (void *)0 defined. > >On Solaris [...] Never mind. - Jan Engelhardt |
Thanks for helping keep SourceForge clean.
X