Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > Code review requested

Reply
Thread Tools

Code review requested

 
 
Keith Thompson
Guest
Posts: n/a
 
      09-24-2012
David Brown <> writes:
> On 24/09/2012 09:26, Keith Thompson wrote:
>> I see a typedef as extra effort to hide the fact that the type
>> is a struct. And the plethora of different conventions (or lack
>> thereof) about the relationship between the struct tag and the
>> typedef name can cause confusion (though that can be avoided by
>> consistenly using the same identifier for both).
>>
>> The case against typedefs for struct types isn't as strong as the
>> case against typedefs for pointer types, but they're similar.

>
> (This is getting a bit off-topic, but I'll ask anyway.)
>
> Why would you have a case against typedefs for pointer types? You
> seem to have a general dislike of typedefs!
>
> I find pointer typedefs are often useful - and in some cases they are
> essential. I don't meant they are essential to writing code that will
> compile - but they are sometimes essential to writing code that is
> clear and unambiguous to the reader and the writer.


I've seen numerous articles here arguing against the use of typedefs for
pointer types. (I've also seen Windows code that uses them extensively.)

The semantics of pointer types and non-pointer types are so different
that I think it's important to be very explicit about which one you're
using. And "some_type*" is a sufficiently simple concept (for someone
familiar with C syntax and semantics) that I don't see much point in
hiding it behind a new name. It already has a name: "some_type*".

C's declaration syntax is admittedly obscure at times; the fact that the
type of a pointer to an array of 10 ints, is "int(*)[10]" is annoying.
But if I were using such a type, I'd consider typedef'ing the array
type, not the pointer type:

typedef int arr_10[10];
arr_10 *ptr;

I wouldn't quite say that I have a general dislike of typedefs.
I just think they're overused. If a typedef creates a truly abstract
type like "FILE", I have no problem with it. If it creates a name
for a type, when the underlying type can vary from one implementation
to another, or if the choice of name adds significant information,
that's fine too; examples are size_t and int32_t.

I don't necessarily *like* the way C defines types, but I'd rather
deal with that than add a layer on top of it to make it look *almost*
like what I would have preferred.

> In my work, const and volatile qualifiers are common, so it is not
> uncommon for the type of a pointer to contain many parts. This makes
> it hard to read, and easy to get wrong:
>
> uint8_t volatile * const p = ...;
>
> Is that a constant pointer to volatile data, or a volatile pointer to
> constant data, or a pointer to a volatile constant?
>
> typedef volatile uint8_t *pVolUint8_t;
> const pVolUint8_t p = ...;
>
> Here there is no doubt what is volatile, and what is const, and how
> the parts are related.


That's a good point. Personally, I haven't run into many cases where
that's much of an issue.

[...]

> Typedefs are even more important if you are talking about function pointers.


An alternative is to typedef the function type, and then use the
explicit "*" for the pointer-to-function type. I don't expect to
convince you that that's better.

> Naming conventions are also very useful here - all my pointer types
> being with "p" (or "P" if you prefer), and all function pointer types
> begin with "f" (or "F" if you prefer).


That's fine until you mix your code with code written by somebody else
who doesn't follow your conventions -- such as the C standard library.

--
Keith Thompson (The_Other_Keith) kst- <http://www.ghoti.net/~kst>
Will write code for food.
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
 
Reply With Quote
 
 
 
 
Nick Keighley
Guest
Posts: n/a
 
      09-25-2012
On Sep 24, 5:28*pm, David Brown <da...@westcontrol.removethisbit.com>
wrote:
> On 24/09/2012 18:16, Nick Keighley wrote:
>
>
>
>
>
> > On Sep 24, 10:05 am, David Brown <da...@westcontrol.removethisbit.com>
> > wrote:
> >> On 24/09/2012 09:26, Keith Thompson wrote:

>
> >>> I see a typedef as extra effort to hide the fact that the type
> >>> is a struct. *And the plethora of different conventions (or lack
> >>> thereof) about the relationship between the struct tag and the
> >>> typedef name can cause confusion (though that can be avoided by
> >>> consistenly using the same identifier for both).

>
> >>> The case against typedefs for struct types isn't as strong as the
> >>> case against typedefs for pointer types, but they're similar.

>
> >> (This is getting a bit off-topic, but I'll ask anyway.)

>
> >> Why would you have a case against typedefs for pointer types? *You seem
> >> to have a general dislike of typedefs!

>
> >> I find pointer typedefs are often useful - and in some cases they are
> >> essential.

>
> > I've never found them essential.

>
> >> * I don't meant they are essential to writing code that will
> >> compile - but they are sometimes essential to writing code that is clear
> >> and unambiguous to the reader and the writer.

>
> >> In my work, const and volatile qualifiers are common, so it is not
> >> uncommon for the type of a pointer to contain many parts. *This makes it
> >> hard to read, and easy to get wrong:

>
> > I don't work in an environment where volatile is common so i may not
> > face the same problems as you

>
> >> * * * * *uint8_t volatile * const p = ...;

>
> >> Is that a constant pointer to volatile data, or a volatile pointer to
> >> constant data, or a pointer to a volatile constant?

>
> >> typedef volatile uint8_t *pVolUint8_t;
> >> const pVolUint8_t p = ...;

>
> > bletch

>
> >> Here there is no doubt what is volatile, and what is const, and how the
> >> parts are related.

>
> >> I seldom have more than two "parts" to a type (either in usage, or as
> >> part of a typedef), and never more than three - typedefs let you put it
> >> together step by clear and unambiguous step.

>
> >> Typedefs are even more important if you are talking about function pointers.

>
> > I'll agree typedefs are necessary for function pointers

>
> >> Naming conventions are also very useful here - all my pointer types
> >> being with "p" (or "P" if you prefer), and all function pointer types
> >> begin with "f" (or "F" if you prefer).

>
> > ug. Hungarian.

>
> No, it's not Hungarian - neither the original (fairly sensible)
> Hungarian notation, nor the really unpleasant misunderstanding that
> people often think of as Hungarian notation. *It's just a helpful
> indication that makes some code clearer - and there is no significant
> difference from calling your "pointer to a compare function" "fCompare"
> or "compareFunc".


to me it seems odd to hide the "pointerness" by using a typedef and
then revealing the pointerness by using an obscure naming convention.
 
Reply With Quote
 
 
 
 
Walter Banks
Guest
Posts: n/a
 
      10-01-2012


Francois Grieu wrote:

> ? If the condition does
> ? not actually need to be re-evaluated, most modern compilers can be
> ? counted on to detect that fact (more reliably than I can), and use an
> ? unnamed temporary for the same purpose that a named boolean variable
> ? would have been used for.
>
> I wish I knew any compiler than could do that for the 8-bit targets
> (ST7-on-steroids, 8051-siblings, PIC18-and-then-some) that I often code for.


In our 8 bitcompilers we detect common conditional expressions but
don't ever save the results for future use for three reasons

1) It doesn't happen very often in real application code.
2) It slows down the original evaluation of the expression usually by interfering with data flow in a primary
register
3) Indirect paths to terms that are not declared as volatiles and should be.

It is one of those optimizations where the programmer has to be disciplined and results are limited and the
support is high. It comes up from time to time in internal product reviews and then we review the metrics of
actual applications and discover that it would be rarely useful

> I doubt it would use bit variables
> and instructions often availabl. Hence, *if* speed or size is more relevant
> than clarity, using an explicit intermediary bit variable would typically be
> beneficial on these targets.


We use bit instructions in our code generation and support single bit booleans on all processors with these
instructions. Our ram allocator packs packs booleans into bytes if we can.

Walter Banks
Byte Craft Limited

 
Reply With Quote
 
David Thompson
Guest
Posts: n/a
 
      10-11-2012
On Thu, 20 Sep 2012 13:29:54 -0700 (PDT), Bart Vandewoestyne
<> wrote:

> On Thursday, September 20, 2012 9:19:11 PM UTC+2, Ian Collins wrote:
> >
> > Rather than assert(0), I prefer assert(!"Some useful diagnostic"). This
> > helps you when it fires and documents why the assert is there in the code.

>

(Below was one huge line, qp'ed. Yuck. Googrope strikes again. Please
see if there is some way to fix this.)

> I have been thinking of this, but i picked assert(0) because I thought

assert(!"Some useful diagnostic") is not standard-conforming. However,
if I compile with gcc and the -std=c99 option i get no warnings, and
thinking about it, the expression !"Some useful diagnostic" probably
also converts to a 'scalar expression' as required by the C99
standard, so is probably completely standard-conforming (please
correct me if I'm wrong).
>


OT1H, that's not strong evidence. Even with -std= (or -ansi), gcc is
not very aggressive about warnings. To get the most help it can give
you, use at least -Wall and -pedantic, and consider -Wextra. Even
then, there are some errors a compiler can't detect and diagnose.

However, it is indeed standard in C99+. Formally it wasn't in C89;
that used a pseudo-prototype void assert(int expression) which was
unclear for pointers and actually wrong for f.p. But in practice
everyone implemented it the traditional and well-known right way.

 
Reply With Quote
 
Philip Lantz
Guest
Posts: n/a
 
      10-11-2012
David Thompson wrote:
>
> Bart Vandewoestyne wrote:
> > I have been thinking of this, but i picked assert(0) because I
> > thought assert(!"Some useful diagnostic") is not standard-
> > conforming. However, thinking about it, the expression
> > !"Some useful diagnostic" probably also converts to a
> > 'scalar expression' as required by the C99 standard, so is
> > probably completely standard-conforming (please correct me
> > if I'm wrong).

>
> ... it is indeed standard in C99+. Formally it wasn't in C89;
> that used a pseudo-prototype void assert(int expression) which was
> unclear for pointers and actually wrong for f.p.


!"string" and !fp are int and always have been, so they were fine to use
with assert even in C89.

I agree that assert("string") wasn't allowed by C89, but that's pretty
useless anyway. assert(fp) was problematic, because it was allowed but
likely wouldn't do what the programmer wanted.
 
Reply With Quote
 
Bart Vandewoestyne
Guest
Posts: n/a
 
      10-19-2012
On Thursday, September 20, 2012 12:44:55 PM UTC+2, Ben Bacarisse wrote:
>
> [...]
> My only big comment... A lot of the code is boilerplate to deal with
> a large number of structure types (well, unions within a structure). I
> would probably search for some way to simplify this. I don't know the
> book, so it's possible you have to do it this way.


I know this thread is from a while ago, but i remember quite a heavy discussion about the structs... Today, I came across an article in Dr. Dobb's with the title 'Discriminated Unions': http://www.drdobbs.com/cpp/discrimin...0009296?pgno=2

It seems to me that the author of the code from Appel's 'Modern Compiler Construction in C' heavily uses this technique... Personally, as quite a C-novice, I didn't know it... but since Dan Saks wrote a blogpost on it, I canimagine that discriminated unions is quite a common thing used in C programming. Searching for 'discriminated' in the latest C standard doesn't giveany results. Does anybody know what book introduced the term 'discriminated unions'? I would like to read more about it...

Regards,
Bart
 
Reply With Quote
 
Ben Bacarisse
Guest
Posts: n/a
 
      10-19-2012
Bart Vandewoestyne <> writes:

> On Thursday, September 20, 2012 12:44:55 PM UTC+2, Ben Bacarisse wrote:
>>
>> [...]
>> My only big comment... A lot of the code is boilerplate to deal with
>> a large number of structure types (well, unions within a structure). I
>> would probably search for some way to simplify this. I don't know the
>> book, so it's possible you have to do it this way.

>
> I know this thread is from a while ago, but i remember quite a heavy
> discussion about the structs... Today, I came across an article in
> Dr. Dobb's with the title 'Discriminated Unions':
> http://www.drdobbs.com/cpp/discrimin...0009296?pgno=2
>
> It seems to me that the author of the code from Appel's 'Modern
> Compiler Construction in C' heavily uses this technique...
> Personally, as quite a C-novice, I didn't know it... but since Dan
> Saks wrote a blogpost on it, I can imagine that discriminated unions
> is quite a common thing used in C programming. Searching for
> 'discriminated' in the latest C standard doesn't give any results.


It goes by other names (tagged unions, for example) but, either way, you
won't find a reference to it in the C standard because it's not a C
language feature. C is a relatively low-level language, so to get the
same effect you have to do the work yourself and make sure that your
code does not "break the rules".

> Does anybody know what book introduced the term 'discriminated
> unions'? I would like to read more about it...


I can't help here, but I can point out that the term is not universal.
For example, Pascal had them, but they are called variant records.
Algol 68 had them and they are just called unions! (I say, had simply
because these languages are old -- they still have all the features they
ever had of course.)

--
Ben.
 
Reply With Quote
 
Bart Vandewoestyne
Guest
Posts: n/a
 
      10-19-2012
On Friday, October 19, 2012 1:24:00 PM UTC+2, Ben Bacarisse wrote:
>
> It goes by other names (tagged unions, for example) but, either way, you
> won't find a reference to it in the C standard because it's not a C
> language feature. C is a relatively low-level language, so to get the
> same effect you have to do the work yourself and make sure that your
> code does not "break the rules".


Indeed. It seems like the concept of 'tagged unions' is more like a computer science abstract data type kind of thing. See e.g. the article on wikipedia:

http://en.wikipedia.org/wiki/Tagged_union

Different languages seem to have different ways of implementing tagged unions, and the way it's done in C is just one of them.

> > Does anybody know what book introduced the term 'discriminated
> > unions'? I would like to read more about it...

>
> I can't help here, but I can point out that the term is not universal.
> For example, Pascal had them, but they are called variant records.
> Algol 68 had them and they are just called unions! (I say, had simply
> because these languages are old -- they still have all the features they
> ever had of course.)


Doing a search on books.google.com leads me to page 44 of the book 'C programming FAQs: frequently asked questions' where tagged unions are shortly mentioned in an answer to a question. Other than that, I can't really find much C-specific books that mention tagged unions. However, there do appear a lot of Data Structures books in the search results...

Interesting. I learned something today!

Regards,
Bart
 
Reply With Quote
 
Keith Thompson
Guest
Posts: n/a
 
      10-19-2012
Bart Vandewoestyne <> writes:
[...]
> Doing a search on books.google.com leads me to page 44 of the book 'C
> programming FAQs: frequently asked questions' where tagged unions are
> shortly mentioned in an answer to a question. Other than that, I
> can't really find much C-specific books that mention tagged unions.
> However, there do appear a lot of Data Structures books in the search
> results...


An abbreviated version of that book is available at
<http://www.c-faq.com/>; it looks like you're referring to question
2.21. It's an extraordinarily useful web site.

--
Keith Thompson (The_Other_Keith) kst- <http://www.ghoti.net/~kst>
Will write code for food.
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
 
Reply With Quote
 
Tim Rentsch
Guest
Posts: n/a
 
      12-17-2012
James Kuyper <> writes:

> On 09/24/2012 03:41 PM, Ian Collins wrote:
>> On 09/25/12 04:16, Nick Keighley wrote:
>>> On Sep 24, 10:05 am, David Brown<da...@westcontrol.removethisbit.com>
>>> wrote:

> ...
>>>> I find pointer typedefs are often useful - and in some cases they are
>>>> essential.
>>>
>>> I've never found them essential.

>>
>>
>> The are in the case of an opaque type.

>
> Could you expound on that a little more? I've written code using opaque
> types that were always referred to in the user code as struct
> myOpaqueType*, with no definition provided about what the members of
> that struct were. I made no use of a typedef while writing such code -
> was I doing something wrong?


If an interface declares, say, a parameter to be of type
'struct myOpaqueType*', then the type of the parameter is
not opaque - it's immediately evident that the parameter is
a pointer, and what the pointer is pointing to is a struct.
For another example, suppose there is a header file that
has

struct example_1 { ... };
extern struct example_1 some_examples[];

The type of some_examples (ie, of the actual object) is not
known, because the array bound isn't known, but certainly
we wouldn't say the type of some_examples is opaque. To be
opaque, the type used on the declaration in question should
convey /no information/ about the type of whatever is being
declared.
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
A little light code review requested luserXtrog C Programming 2 08-18-2010 04:15 AM
Re: Code review requested, Accelerated C++ Vyacheslav Kononenko C++ 0 10-18-2004 01:58 PM
Re: Code review requested, Accelerated C++ Mike Wahler C++ 1 10-18-2004 03:43 AM
Code review requested Ed Java 1 07-19-2004 12:11 PM
Code review requested Ian Pilcher Java 0 12-15-2003 04:53 AM



Advertisments
 



1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57