Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c > #147078 > unrolled thread

How to use a struct with qsort()?

Back to article view | Back to comp.lang.c

Contents

  How to use a struct with qsort()? DFS <nospam@dfs.com> - 2019-10-19 13:35 -0400
    Re: How to use a struct with qsort()? Malcolm McLean <malcolm.arthur.mclean@gmail.com> - 2019-10-19 10:22 -0700
      Re: How to use a struct with qsort()? DFS <nospam@dfs.com> - 2019-10-19 14:19 -0400
        Re: How to use a struct with qsort()? Ben Bacarisse <ben.usenet@bsb.me.uk> - 2019-10-19 19:36 +0100
        Re: How to use a struct with qsort()? Melzzzzz <Melzzzzz@zzzzz.com> - 2019-10-19 18:44 +0000
    Re: How to use a struct with qsort()? Barry Schwarz <schwarzb@delq.com> - 2019-10-19 10:23 -0700
    Re: How to use a struct with qsort()? Ike Naar <ike@sdf.lonestar.org> - 2019-10-19 19:09 +0000
      Re: How to use a struct with qsort()? DFS <nospam@dfs.com> - 2019-10-19 15:51 -0400
        Re: How to use a struct with qsort()? already5chosen@yahoo.com - 2019-10-19 13:09 -0700
          Re: How to use a struct with qsort()? DFS <nospam@dfs.com> - 2019-10-19 16:26 -0400
            Re: How to use a struct with qsort()? already5chosen@yahoo.com - 2019-10-19 13:51 -0700
              Re: How to use a struct with qsort()? DFS <nospam@dfs.com> - 2019-10-19 17:24 -0400
                Re: How to use a struct with qsort()? already5chosen@yahoo.com - 2019-10-19 14:49 -0700
                  Re: How to use a struct with qsort()? DFS <nospam@dfs.com> - 2019-10-19 18:26 -0400
                    Re: How to use a struct with qsort()? already5chosen@yahoo.com - 2019-10-19 16:32 -0700
                      Re: How to use a struct with qsort()? DFS <nospam@dfs.com> - 2019-10-20 09:22 -0400
                        Re: How to use a struct with qsort()? Paul <nospam@needed.invalid> - 2019-10-26 04:15 -0400
                          Re: How to use a struct with qsort()? DFS <nospam@dfs.com> - 2019-10-26 09:37 -0400
                            Re: How to use a struct with qsort()? Bart <bc@freeuk.com> - 2019-10-26 15:40 +0100
                              Re: How to use a struct with qsort()? DFS <nospam@dfs.com> - 2019-10-28 19:49 -0400
                                Re: How to use a struct with qsort()? Paul <nospam@needed.invalid> - 2019-10-29 01:59 -0400
                                  Re: How to use a struct with qsort()? Paul <nospam@needed.invalid> - 2019-10-30 04:54 -0400
                                    Re: How to use a struct with qsort()? Bart <bc@freeuk.com> - 2019-10-30 11:54 +0000
                                      Re: How to use a struct with qsort()? Keith Thompson <kst-u@mib.org> - 2019-10-30 08:58 -0700
                                      Re: How to use a struct with qsort()? Öö Tiib <ootiib@hot.ee> - 2019-11-07 01:10 -0800
                                        Re: How to use a struct with qsort()? Bart <bc@freeuk.com> - 2019-11-07 10:51 +0000
                            Re: How to use a struct with qsort()? Paul <nospam@needed.invalid> - 2019-10-26 15:03 -0400
                              Re: How to use a struct with qsort()? Ben Bacarisse <ben.usenet@bsb.me.uk> - 2019-10-26 21:50 +0100
                          Re: How to use a struct with qsort()? Tim Rentsch <tr.17687@z991.linuxsc.com> - 2019-11-04 16:02 -0800
              Re: How to use a struct with qsort()? Jorgen Grahn <grahn+nntp@snipabacken.se> - 2019-10-21 11:58 +0000
              Re: How to use a struct with qsort()? Tim Rentsch <tr.17687@z991.linuxsc.com> - 2019-10-23 04:51 -0700
            Re: How to use a struct with qsort()? Bart <bc@freeuk.com> - 2019-10-19 23:25 +0100
              Re: How to use a struct with qsort()? Keith Thompson <kst-u@mib.org> - 2019-10-19 15:38 -0700
                Re: How to use a struct with qsort()? David Brown <david.brown@hesbynett.no> - 2019-10-20 14:44 +0200
                Re: How to use a struct with qsort()? DFS <nospam@dfs.com> - 2019-10-22 15:42 -0400
                Re: How to use a struct with qsort()? Tim Rentsch <tr.17687@z991.linuxsc.com> - 2019-10-23 05:47 -0700
                  Re: How to use a struct with qsort()? Bart <bc@freeuk.com> - 2019-10-23 16:40 +0100
                    Re: How to use a struct with qsort()? Keith Thompson <kst-u@mib.org> - 2019-10-23 13:51 -0700
                      Re: How to use a struct with qsort()? Bart <bc@freeuk.com> - 2019-10-23 23:07 +0100
                        Re: How to use a struct with qsort()? Keith Thompson <kst-u@mib.org> - 2019-10-23 15:50 -0700
                          Re: How to use a struct with qsort()? Bart <bc@freeuk.com> - 2019-10-24 01:20 +0100
                            Re: How to use a struct with qsort()? Keith Thompson <kst-u@mib.org> - 2019-10-23 18:20 -0700
                              Re: How to use a struct with qsort()? Bart <bc@freeuk.com> - 2019-10-24 10:40 +0100
                                Re: How to use a struct with qsort()? Keith Thompson <kst-u@mib.org> - 2019-10-24 11:00 -0700
            Re: How to use a struct with qsort()? Ben Bacarisse <ben.usenet@bsb.me.uk> - 2019-10-20 00:30 +0100
              Re: How to use a struct with qsort()? Bart <bc@freeuk.com> - 2019-10-20 00:44 +0100
            Re: How to use a struct with qsort()? Ben Bacarisse <ben.usenet@bsb.me.uk> - 2019-10-20 01:27 +0100
            Re: How to use a struct with qsort()? Manfred <noname@add.invalid> - 2019-10-25 20:30 +0200
              Re: How to use a struct with qsort()? DFS <nospam@dfs.com> - 2019-10-25 16:07 -0400
              Re: How to use a struct with qsort()? Ike Naar <ike@sdf.lonestar.org> - 2019-10-26 07:27 +0000
                Re: How to use a struct with qsort()? Keith Thompson <kst-u@mib.org> - 2019-10-26 02:11 -0700
              Re: How to use a struct with qsort()? Bart <bc@freeuk.com> - 2019-10-26 11:06 +0100
                Re: How to use a struct with qsort()? Manfred <noname@invalid.add> - 2019-10-26 16:58 +0200
                  Re: How to use a struct with qsort()? DFS <nospam@dfs.com> - 2019-10-26 18:17 -0400
                    Re: How to use a struct with qsort()? Keith Thompson <kst-u@mib.org> - 2019-10-26 16:08 -0700
                      Re: How to use a struct with qsort()? DFS <nospam@dfs.com> - 2019-10-26 21:33 -0400
                        Re: How to use a struct with qsort()? Keith Thompson <kst-u@mib.org> - 2019-10-26 22:10 -0700
                    Re: How to use a struct with qsort()? Ben Bacarisse <ben.usenet@bsb.me.uk> - 2019-10-27 01:48 +0100
                    Re: How to use a struct with qsort()? Manfred <noname@invalid.add> - 2019-10-27 19:29 +0100
                    Re: How to use a struct with qsort()? Barry Schwarz <schwarzb4aarp@gmail.com> - 2019-10-27 13:09 -0700

Page 3 of 3 — ← Prev page 1 2 [3]

#147181

On 23/10/2019 23:50, Keith Thompson wrote:
> Bart <bc@freeuk.com> writes:

> One of your examples was
>      struct T T;
> It could have been
>      struct COFF_Reloc_Rec COFF_Reloc_Rec;
> Do we agree that using the same identifer for a struct tag and for an
> object of the struct type is a bad idea?  Or for a typedef name and an
> object of that type?  That's what you did in your example.  Perhaps a
> longer name would have made that clearer.

Yet C allows that, and in the same scope. That's only possible using 
struct (and enum) tags, so avoiding struct tags makes it impossible to 
do that.

> I don't recall anyone suggesting using the same name for a type and
> for an object of that type.  You did so in your example.  If that
> wasn't your intent, I'll accept that -- but then I'm not sure what point
> you were making.

In the original example, 'struct words' was used, with 'wordrec' used 
for a variable, even though 'words' would have been a better name for 
the variable. Yet the language allows, via struct tags, both 'struct 
words' and the variable 'words' to be used together.

> Yes, I understand that.  "int*" is not an anonymous type.  It has a
> name, and that name is "int*".  (See the definition of "type-name" in
> the C syntax.  It's not just an identifier.)

As I said, the context here was user-names for struct tags, typedefs and 
variables. I don't know what unambiguous denotation can be used for each 
of those to make it clear I'm not talking about type constructs.

> Great.  In this case, doing so may have unintentionally obscured your
> point.

Really? You see 'struct T T' and you should instantly see the problem.

[toc] | [prev] | [next] | [standalone]

#147183

Bart <bc@freeuk.com> writes:
> On 23/10/2019 23:50, Keith Thompson wrote:
>> One of your examples was
>>      struct T T;
>> It could have been
>>      struct COFF_Reloc_Rec COFF_Reloc_Rec;
>> Do we agree that using the same identifer for a struct tag and for an
>> object of the struct type is a bad idea?  Or for a typedef name and an
>> object of that type?  That's what you did in your example.  Perhaps a
>> longer name would have made that clearer.
>
> Yet C allows that, and in the same scope. That's only possible using 
> struct (and enum) tags, so avoiding struct tags makes it impossible to 
> do that.

Yes, C allows all kinds of things that are bad ideas.  All languages do.

You haven't answered my question.  Do you agree that using the same name
for an object and the tag for the object's name is a bad idea?  I
believe you do, but I don't want to put words in your mouth.

Are you saying that (a) you dislike struct tags, and (b) the reason you
dislike struct tags is that they make it possible to use the same
identifier for an object and for the tag for the object's type?  I can
understand (a).  If you're claiming (b), I find that very odd.

>> I don't recall anyone suggesting using the same name for a type and
>> for an object of that type.  You did so in your example.  If that
>> wasn't your intent, I'll accept that -- but then I'm not sure what point
>> you were making.
>
> In the original example, 'struct words' was used, with 'wordrec' used 
> for a variable, even though 'words' would have been a better name for 
> the variable. Yet the language allows, via struct tags, both 'struct 
> words' and the variable 'words' to be used together.
> 
>> Yes, I understand that.  "int*" is not an anonymous type.  It has a
>> name, and that name is "int*".  (See the definition of "type-name" in
>> the C syntax.  It's not just an identifier.)
>
> As I said, the context here was user-names for struct tags, typedefs and 
> variables. I don't know what unambiguous denotation can be used for each 
> of those to make it clear I'm not talking about type constructs.

I suppose you're talking about "type names that are either a single
identifier or the keyword struct, union, or enum followed by a single
identifier".  So what point are you making about such type names?

>> Great.  In this case, doing so may have unintentionally obscured your
>> point.
>
> Really? You see 'struct T T' and you should instantly see the problem.

I see two problems.  One is the use of the non-descriptive name "T",
which we've thoroughly covered.  The other is the use of the same
identifier as a tag and as an object name.

You make the point that using a struct tag makes it possible to
use the same identifier for a struct tag and for an object of the
struct type.  I agree (though I don't agree that it's a big deal,
since it can easily be avoided by picking sensible names).  Did you
have another point?

I don't intend to continue posting on this thread for much longer, so
please make any points you wish me to respond to as soon as possible.

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
Will write code for food.
void Void(void) { Void(); } /* The recursive call of the void */

[toc] | [prev] | [next] | [standalone]

#147188

On 24/10/2019 02:20, Keith Thompson wrote:
> Bart <bc@freeuk.com> writes:

>> Really? You see 'struct T T' and you should instantly see the problem.
> 
> I see two problems.  One is the use of the non-descriptive name "T",
> which we've thoroughly covered.

Yes. It is totally irrelevant, and is chosen to highlight the actual 
point that exactly the same identifier can be used for very different 
purposes. I still fail to understand why you mentioned it in the first 
place.

If I was talking about the following ways of using arrays in C:

    int A[10];
    int *B = malloc(40);
    int (*C)[]=malloc(40);

would you also be sidetracked by the fact that A, B, C are 
non-descriptive single letter names?


   The other is the use of the same
> identifier as a tag and as an object name.
> 
> You make the point that using a struct tag makes it possible to
> use the same identifier for a struct tag and for an object of the
> struct type.  I agree (though I don't agree that it's a big deal,
> since it can easily be avoided by picking sensible names).

You wouldn't. A beginner might, since there is nothing that stops it.


> Did you
> have another point?

Did you have another pointless objection?

[toc] | [prev] | [next] | [standalone]

#147203

Bart <bc@freeuk.com> writes:
> On 24/10/2019 02:20, Keith Thompson wrote:
>> Bart <bc@freeuk.com> writes:
>
>>> Really? You see 'struct T T' and you should instantly see the problem.
>> 
>> I see two problems.  One is the use of the non-descriptive name "T",
>> which we've thoroughly covered.
>
> Yes. It is totally irrelevant, and is chosen to highlight the actual 
> point that exactly the same identifier can be used for very different 
> purposes. I still fail to understand why you mentioned it in the first 
> place.

I fail to understand why we're still talking about it.  We've been
agreeing on this point for the last several followups.

>> The other is the use of the same identifier as a tag and as an object
>> name.
>> 
>> You make the point that using a struct tag makes it possible to
>> use the same identifier for a struct tag and for an object of the
>> struct type.  I agree (though I don't agree that it's a big deal,
>> since it can easily be avoided by picking sensible names).
>
> You wouldn't. A beginner might, since there is nothing that stops it.
>
>> Did you have another point?
>
> Did you have another pointless objection?

If you're not going to answer straightforward questions, this
discussion is over.

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
Will write code for food.
void Void(void) { Void(); } /* The recursive call of the void */

[toc] | [prev] | [next] | [standalone]

#147098

DFS <nospam@dfs.com> writes:

> Here's the program as it stands, which works great except for
> this sorting issue.  You can test it by using words_alpha.txt from
> https://github.com/dwyl/english-words

Just a heads-up: you've excluded people who can't build MS-Windows
programs.  I'm not sure why since the task appears to be a simple text
processing one, but some people won't bother with a Windows build.

Anyway, a comment or two...

> //build struct
> struct words {char *word; double wordpf;};
> struct words **wordrec = NULL;

You want a simple "struct words *wordrec" here.  You can get by with an
array of pointers, but I can't see the point.

> //sort structure by prime factors, then word
> int comparepf(const void *p1, const void *p2)
> {
>     const struct words *a = p1;
>     const struct words *b = p2;
>     //printf("%.0lf  %.0lf\n",a->wordpf,b->wordpf);
>     if(a->wordpf > b->wordpf)
> 		return 1;
> 	else if (a->wordpf < b->wordpf)
> 		return -1;
>     else
> 		return 0;
> }

This function is wrong if the array is of struct pointers.  It is
written for my preferred solution: an array of structs.  Therefore,
though it might be simplest to correct this function, I'd change the
other parts instead.

> 		//memory
> 		wordrec = (struct words **)realloc(wordrec,(wordcnt+1) * sizeof(struct words *));

There's a lot to be said about this line, but let's just fix a couple of
things:

  wordrec = realloc(wordrec, (wordcnt+1) * sizeof *wordrec);

i.e. drop the cast and use the size of the pointer rather than repeating
what type it is.  Had you done this originally, you could change the
type of wordrec as I am suggesting above and not have to bother about
this line.

> 		wordrec[wordcnt] = (struct words *)malloc(sizeof(struct words));

If you change wordrec to be an array of structs you don't need this line
at all!

> 		wordrec[wordcnt]->word  = malloc(strlen(wordin)+1);

and this line becomes

  wordrec[wordcnt].word = malloc(strlen(wordin)+1);

> 	qsort(struct wordrec, wordcnt, sizeof(struct wordrec), comparepf);

This won't compile.  If you make the changes I suggest, you'll need

  qsort(wordrec, wordcnt, sizeof *wordrec, comparepf);

But if keep wordrec as an array of pointers you have the element size
wrong.  Each one is sizeof(struct wordrec *) not sizeof(struct
wordrec).  You can avoid this problem by always using the size of data
itself so you never have to write the type put again.  In other words

  qsort(wordrec, wordcnt, sizeof *wordrec, comparepf);

will work for an array of structs and for an array of pointers to
structs.  The compare function has to be written differently, but
neither the allocation lines nor the qsort line needs to change if you
swap to an array of structs.

-- 
Ben.

[toc] | [prev] | [next] | [standalone]

#147100

On 20/10/2019 00:30, Ben Bacarisse wrote:
> DFS <nospam@dfs.com> writes:
> 
>> Here's the program as it stands, which works great except for
>> this sorting issue.  You can test it by using words_alpha.txt from
>> https://github.com/dwyl/english-words
> 
> Just a heads-up: you've excluded people who can't build MS-Windows
> programs.  I'm not sure why since the task appears to be a simple text
> processing one, but some people won't bother with a Windows build.

Removing Windows and timing related code (QueryPerformance, elapsedtime 
etc) makes it compile under Linux (with a fix for line 111).

(Such timing is probably premature here, but I anyway just use clock(), 
or just time the whole program.)

[toc] | [prev] | [next] | [standalone]

#147102

DFS <nospam@dfs.com> writes:

> Here's the program as it stands, which works great except for
> this sorting issue.

You can make it work by removing the sort line!  It turns out you don't
use the fact that the list is sorted.  Now that might be a temporary
issue.  If so, you need to take a look at the bsearch function.

I was able to get you code to work (after removing the Windows stuff)
with only two changes.  (1) Correct the compare function so that it
expects two pointer to pointer arguments; and (2) correct the qsort line
as I showed before.  There are other issues (missing ctype.h for
example) but this was the minimum change.

It's not my preferred correction (you don't need pointers to structs in
the array) but it does work.

> //sort structure by prime factors, then word
> int comparepf(const void *p1, const void *p2)
> {
>     const struct words *a = p1;
>     const struct words *b = p2;

Put

  const struct words **ap = p1, *a = *ap;
  const struct words **bp = p2, *b = *bp;

here to get started.
     
>     //printf("%.0lf  %.0lf\n",a->wordpf,b->wordpf);
>     if(a->wordpf > b->wordpf)
> 		return 1;
> 	else if (a->wordpf < b->wordpf)
> 		return -1;
>     else
> 		return 0;
> }

> 	qsort(struct wordrec, wordcnt, sizeof(struct wordrec), comparepf);

  qsort(wordrec, wordcnt, sizeof *wordrec, comparepf);

-- 
Ben.

[toc] | [prev] | [next] | [standalone]

#147232

On 10/19/2019 10:26 PM, DFS wrote:
> int primes[26] = {2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41,
>           43, 47, 53, 59, 61, 67, 71, 73, 79, 83, 89, 97, 101}; 
> char *charset = "abcdefghijklmnopqrstuvwxyz";
Since primes and charset are strictly coupled, it is a good idea to 
enforce consistency of their sizes as in:
#include <assert.h>
static const unsigned int primes[] =
{
   2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41,
   43, 47, 53, 59, 61, 67, 71, 73, 79, 83, 89, 97, 101
};
static const char charset[] = "abcdefghijklmnopqrstuvwxyz";
static_assert(sizeof(primes)/sizeof(primes[0]) == 
(sizeof(charset)/sizeof(charset[0])-1),
     "charset and primes mismatch");

[...]

> //compute product of primes corresponding to the letters of the input word
> double calcpf(char *str, int len) {
>      double pf = 1;
>      for(i=0;i<len;i++) {

FWIW the following will result in an access violation whenever str[i] is 
not found in charset (e.g for an uppercase char). Since str comes from 
an external source (the dictionary file), the program should be able to 
handle this possibility.
In general, it is not a good idea to index into an array so blindly.
>          pf *= primes[(int)(strchr(charset, str[i]) - charset)];

>      }
>      return pf;
> }

[toc] | [prev] | [next] | [standalone]

#147233

On 10/25/2019 2:30 PM, Manfred wrote:
> On 10/19/2019 10:26 PM, DFS wrote:
>> int primes[26] = {2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41,
>>           43, 47, 53, 59, 61, 67, 71, 73, 79, 83, 89, 97, 101}; char 
>> *charset = "abcdefghijklmnopqrstuvwxyz";
> Since primes and charset are strictly coupled, it is a good idea to 
> enforce consistency of their sizes as in:
> #include <assert.h>
> static const unsigned int primes[] =
> {
>    2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41,
>    43, 47, 53, 59, 61, 67, 71, 73, 79, 83, 89, 97, 101
> };
> static const char charset[] = "abcdefghijklmnopqrstuvwxyz";
> static_assert(sizeof(primes)/sizeof(primes[0]) == 
> (sizeof(charset)/sizeof(charset[0])-1),
>      "charset and primes mismatch");
> 
> [...]


Thanks for the tips.



>> //compute product of primes corresponding to the letters of the input 
>> word
>> double calcpf(char *str, int len) {
>>      double pf = 1;
>>      for(i=0;i<len;i++) {



> FWIW the following will result in an access violation whenever str[i] is 
> not found in charset (e.g for an uppercase char). Since str comes from 
> an external source (the dictionary file), the program should be able to 
> handle this possibility.
> In general, it is not a good idea to index into an array so blindly.
>>          pf *= primes[(int)(strchr(charset, str[i]) - charset)];
> 
>>      }
>>      return pf;
>> }


Thanks, but of course I know how to handle incoming vs acceptable data. 
This is just personal code.  The dictionary is local and under my 
control, and is all lowercase.

[toc] | [prev] | [next] | [standalone]

#147237

On 2019-10-25, Manfred <noname@add.invalid> wrote:
> On 10/19/2019 10:26 PM, DFS wrote:
>> int primes[26] = {2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41,
>>           43, 47, 53, 59, 61, 67, 71, 73, 79, 83, 89, 97, 101}; 
>> char *charset = "abcdefghijklmnopqrstuvwxyz";
> Since primes and charset are strictly coupled, it is a good idea to 
> enforce consistency of their sizes as in:
> #include <assert.h>
> static const unsigned int primes[] =
> {
>    2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41,
>    43, 47, 53, 59, 61, 67, 71, 73, 79, 83, 89, 97, 101
> };
> static const char charset[] = "abcdefghijklmnopqrstuvwxyz";
> static_assert(sizeof(primes)/sizeof(primes[0]) == 
> (sizeof(charset)/sizeof(charset[0])-1),
>      "charset and primes mismatch");

Why the "-1" ?
primes[] and charset[] have the same number of elements (26).

[toc] | [prev] | [next] | [standalone]

#147239

Ike Naar <ike@sdf.lonestar.org> writes:
> On 2019-10-25, Manfred <noname@add.invalid> wrote:
>> On 10/19/2019 10:26 PM, DFS wrote:
>>> int primes[26] = {2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41,
>>>           43, 47, 53, 59, 61, 67, 71, 73, 79, 83, 89, 97, 101}; 
>>> char *charset = "abcdefghijklmnopqrstuvwxyz";
>> Since primes and charset are strictly coupled, it is a good idea to 
>> enforce consistency of their sizes as in:
>> #include <assert.h>
>> static const unsigned int primes[] =
>> {
>>    2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41,
>>    43, 47, 53, 59, 61, 67, 71, 73, 79, 83, 89, 97, 101
>> };
>> static const char charset[] = "abcdefghijklmnopqrstuvwxyz";
>> static_assert(sizeof(primes)/sizeof(primes[0]) == 
>> (sizeof(charset)/sizeof(charset[0])-1),
>>      "charset and primes mismatch");
>
> Why the "-1" ?
> primes[] and charset[] have the same number of elements (26).

No, charset[] has 27 elements.  It's initialized from a string literal,
so there's a trailing '\0'.

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
Will write code for food.
void Void(void) { Void(); } /* The recursive call of the void */

[toc] | [prev] | [next] | [standalone]

#147240

On 25/10/2019 19:30, Manfred wrote:
> On 10/19/2019 10:26 PM, DFS wrote:
>> int primes[26] = {2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41,
>>           43, 47, 53, 59, 61, 67, 71, 73, 79, 83, 89, 97, 101}; char 
>> *charset = "abcdefghijklmnopqrstuvwxyz";
> Since primes and charset are strictly coupled, it is a good idea to 
> enforce consistency of their sizes as in:
> #include <assert.h>
> static const unsigned int primes[] =
> {
>    2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41,
>    43, 47, 53, 59, 61, 67, 71, 73, 79, 83, 89, 97, 101
> };
> static const char charset[] = "abcdefghijklmnopqrstuvwxyz";
> static_assert(sizeof(primes)/sizeof(primes[0]) == 

> (sizeof(charset)/sizeof(charset[0])-1),
>      "charset and primes mismatch");


Looks like overkill. Since charset is defined as a 'char' array in the 
preceding line, you can assume that sizeof(charset[0]) is 1.

After all, it does assume that charset[] is initialised from a string, 
so has a "-1" (as pointed out by someone), but it will go wrong if 
initialised from {'a',...'z'};

It also does not report an error if the letters are not in order 
(although for this program it doesn't matter, very often it will do).

Neither will it do so if both primes and charset both have too many 
elements, so long as they both have the same number. Nor when they both 
have too few (you additionally need an assert that prime has 26 elements).

Nor when some elements of prime[] are not prime numbers. Or it has 
duplicates. Or characters inside charset are not a-z or are duplicates.

In short, so many assumptions are being made anyway...

(TBF, use of charset[] is already overkill, since its entire purpose 
seems to be to convert 'a' to 'z' into 0 to 25, which is trivially done 
by subtracting 'a'.

Since the program seems to rely on a word list downloaded from the 
internet, the chances of that file being 8-bit EBCDIC are infinitesimal.)

[toc] | [prev] | [next] | [standalone]

#147244

On 10/26/19 12:06 PM, Bart wrote:
> On 25/10/2019 19:30, Manfred wrote:
>> On 10/19/2019 10:26 PM, DFS wrote:
>>> int primes[26] = {2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41,
>>>           43, 47, 53, 59, 61, 67, 71, 73, 79, 83, 89, 97, 101}; char 
>>> *charset = "abcdefghijklmnopqrstuvwxyz";
>> Since primes and charset are strictly coupled, it is a good idea to 
>> enforce consistency of their sizes as in:
>> #include <assert.h>
>> static const unsigned int primes[] =
>> {
>>    2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41,
>>    43, 47, 53, 59, 61, 67, 71, 73, 79, 83, 89, 97, 101
>> };
>> static const char charset[] = "abcdefghijklmnopqrstuvwxyz";
>> static_assert(sizeof(primes)/sizeof(primes[0]) == 
> 
>> (sizeof(charset)/sizeof(charset[0])-1),
>>      "charset and primes mismatch");
> 
> 
> Looks like overkill. Since charset is defined as a 'char' array in the 
> preceding line, you can assume that sizeof(charset[0]) is 1.

In this particular case it is more verbose than needed, but:
1) The constraint to be enforced is about number of /elements/ of the 
two arrays (modulo '\0' in the string), not /bytes/, so in this way I 
think it is more clear what it is about, even if it happens that element 
size of the second array is 1 - I see this as a coincidence that is 
orthogonal to the algorithm logic.
2) In this way the code will continue to work with /no change/ if at 
some point the character type be changed into something else, typically 
wchar_t (I know that wchar_t is not popular in *nix programming, but it 
is almost a must in any real world Windows programming)
3) You know that this is all compile-time processing, so even if it is 
more verbose it has zero impact on the compiled code.

> 
> After all, it does assume that charset[] is initialised from a string, 
> so has a "-1" (as pointed out by someone), but it will go wrong if 
> initialised from {'a',...'z'};
In the expression as written the -1 will continue to work with wchar_t, 
and yes it will not work if the array is brace initialized, but I think 
that changing a string into a brace list is not something that is easily 
overlooked by any C programmer.

> 
> It also does not report an error if the letters are not in order 
> (although for this program it doesn't matter, very often it will do).
> 
> Neither will it do so if both primes and charset both have too many 
> elements, so long as they both have the same number. Nor when they both 
> have too few (you additionally need an assert that prime has 26 elements).
> 
> Nor when some elements of prime[] are not prime numbers. Or it has 
> duplicates. Or characters inside charset are not a-z or are duplicates.
> 
> In short, so many assumptions are being made anyway...

The main purpose of this static_assert it to guarantee consistency with 
usage of the arrays, i.e. the following calcpf():
> //compute product of primes corresponding to the letters of the input word
> double calcpf(char *str, int len) {
>     double pf = 1;
>     for(i=0;i<len;i++) {
>         pf *= primes[(int)(strchr(charset, str[i]) - charset)];
>     }
>     return pf;
> } 

It is this function that is strictly coupled with the two arrays above, 
and, apart of possible overflow of the multiplication, what is required 
for this function to avoid UB is to guard the expression
> primes[(int)(strchr(charset, str[i]) - charset)];

This expression has defined behavior if and only if:
1) strchr returns a pointer into charset (hence my remark about the need 
for handling this at runtime)
2) charset and primes have the same number of elements[*] (hence the 
static_assert)

Yes, there may be many other mistakes that can be made, but at least the 
examples you mentioned would cause incorrect results rather than 
undefined behavior (and incorrect results are more likely to be caught 
during testing than UB, which is the nastiest of beasts). On the other 
hand, filling the code with static_asserts would really be overkill, and 
not what the expression is meant for.

> 
> (TBF, use of charset[] is already overkill, since its entire purpose 
> seems to be to convert 'a' to 'z' into 0 to 25, which is trivially done 
> by subtracting 'a'.
> 
> Since the program seems to rely on a word list downloaded from the 
> internet, the chances of that file being 8-bit EBCDIC are infinitesimal.)
> 

There can be much more to be said about the complete program (the OP's 
memory management mistakes and his/her remark about books being 
unhelpful suggest limited knowledge of C) - my 2c were on this piece of 
functionality that I thought was worth a comment.

[*] (strictly speaking, even if primes has more elements that charset 
there still is defined behavior.)

[toc] | [prev] | [next] | [standalone]

#147269

On 10/26/2019 10:58 AM, Manfred wrote:


> There can be much more to be said about the complete program 

Please, speak up.  Won't hurt my feelings.


> (the OP's memory management mistakes 

Feel free to show them.  I want to get it as right as possible.


> and his/her remark about books being 
> unhelpful suggest limited knowledge of C) 


Let me show you an example:

Bart posted a modified example of my code.  In it he used memset this way:

#define N_LETTERS 26
unsigned char sc[N_LETTERS];
memset(sc, 0, N_LETTERS);


A tutorialspoint.com webpage does it similarly:
char str[50];
memset(str,'$',7);


Should be simple, right?

So I try it in my code, to replace a small loop:

int wsvals[26]={0};   (not a char as in previous examples)
memset(wsvals,0,26);

Doesn't work.  The code compiles (tcc Windows) with no warnings, but it 
doesn't execute.

Finally on stackoverflow I find this:
"Note that for memset you have to pass the number of bytes, not the 
number of elements because this is an old C function:

So I do:  memset(wsvals,0,26 * sizeof(int));

And it works.

Most of the descriptions on the web say memset "is used to fill a block 
of memory with given/particular value."  No mention of the datatype.

Why do I need to also use sizeof() for an int but not a char?

[toc] | [prev] | [next] | [standalone]

#147271

DFS <nospam@dfs.com> writes:
[...]
> So I try it in my code, to replace a small loop:
>
> int wsvals[26]={0};   (not a char as in previous examples)
> memset(wsvals,0,26);
>
> Doesn't work.  The code compiles (tcc Windows) with no warnings, but it 
> doesn't execute.

I presume it does execute.  It just doesn't do what you wanted it to do.

> Finally on stackoverflow I find this:
> "Note that for memset you have to pass the number of bytes, not the 
> number of elements because this is an old C function:
>
> So I do:  memset(wsvals,0,26 * sizeof(int));
>
> And it works.
>
> Most of the descriptions on the web say memset "is used to fill a block 
> of memory with given/particular value."  No mention of the datatype.
>
> Why do I need to also use sizeof() for an int but not a char?

Because sizeof (char) is 1 by definition.  Given:
    char arr[26];
you could do either
    memset(arr, 0, 26);
or
    memset(arr, 0, 26*sizeof(char));
Both are equivalent.

But given an array of any type that you want to zero-fill, just apply
sizeof to the entire object:
    int wsvals[26];
    memset(wsvals, 0, sizeof wsvals);

Note also that in your example, you already initialized the array:
    int wsvals[26]={0};
so there's no need to call memset.

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
Will write code for food.
void Void(void) { Void(); } /* The recursive call of the void */

[toc] | [prev] | [next] | [standalone]

#147275

On 10/26/2019 7:08 PM, Keith Thompson wrote:
> DFS <nospam@dfs.com> writes:
> [...]
>> So I try it in my code, to replace a small loop:
>>
>> int wsvals[26]={0};   (not a char as in previous examples)
>> memset(wsvals,0,26);
>>
>> Doesn't work.  The code compiles (tcc Windows) with no warnings, but it
>> doesn't execute.
> 
> I presume it does execute.  It just doesn't do what you wanted it to do.

Can't tell what it does.  These are the letter counts /after/ 
memset(vals,0,26), where vals is an int.  The first one is correct 
because it's the first count done after initialization.

abcdefghijklmnopqrstuvwxyz
21011001000100000010000010 abashedly
11001001000200000010000010 able
11000011000200000010000010 bag
11000001000200000011000010 bat
11001001000300000011000010 bela
10100001000300000012000010 cat
10101002000400110022100010 cephalous
11101002100510220033100010 compatibles
11000012100510220033100010 gab
20112103100511420334100030 hydroferrocyanate
10002003200513430434100030 neperian
10001003200613440544100040 parsley
10001003200713450654100050 players
11002004300713460985300050 subtreasurership
11000004300713460986300050 tab
300010143008154601088410160 unextravagantly
100000044008164701088410160 Spain
100000045008174801098410160 pains
101000055008274801198410160 march
101000065008274801298410160 March

now use
memset(vals, 0, 26*sizeof(int))
or
memset(vals, 0, sizeof vals)

abcdefghijklmnopqrstuvwxyz
21011001000100000010000010 abashedly
11001000000100000000000000 able
11000010000000000000000000 bag
11000000000000000001000000 bat
11001000000100000000000000 bela
10100000000000000001000000 cat
10101001000100110010100000 cephalous
11101000100110110011000000 compatibles
11000010000000000000000000 gab
20112101000001200301000020 hydroferrocyanate
10002000100002010100000000 neperian
10001000000100010110000010 parsley
10001000000100010110000010 players
11002001100000010331200000 subtreasurership
11000000000000000001000000 tab
30001010000102000102110110 unextravagantly
10000000100001010000000000 Spain
10000000100001010010000000 pains
10100001000010000100000000 march
10100001000000000100000000 March


>> Finally on stackoverflow I find this:
>> "Note that for memset you have to pass the number of bytes, not the
>> number of elements because this is an old C function:
>>
>> So I do:  memset(wsvals,0,26 * sizeof(int));
>>
>> And it works.
>>
>> Most of the descriptions on the web say memset "is used to fill a block
>> of memory with given/particular value."  No mention of the datatype.
>>
>> Why do I need to also use sizeof() for an int but not a char?
> 
> Because sizeof (char) is 1 by definition.  Given:
>      char arr[26];
> you could do either
>      memset(arr, 0, 26);
> or
>      memset(arr, 0, 26*sizeof(char));
> Both are equivalent.


Why doesn't the compiler recognize the data type of the object being 
passed to memset, and handle the sizing for you?

So that memset(arr, v, N) would work for chars and ints.



> But given an array of any type that you want to zero-fill, just apply
> sizeof to the entire object:
>      int wsvals[26];
>      memset(wsvals, 0, sizeof wsvals);


Perfect.  I'll remember to use that from now on.

memset is probably a bit faster than a loop, too.


> Note also that in your example, you already initialized the array:
>      int wsvals[26]={0};
> so there's no need to call memset.

I didn't show, but it needs to be reset each loop.

[toc] | [prev] | [next] | [standalone]

#147276

DFS <nospam@dfs.com> writes:
> On 10/26/2019 7:08 PM, Keith Thompson wrote:
>> DFS <nospam@dfs.com> writes:
>> [...]
>>> So I try it in my code, to replace a small loop:
>>>
>>> int wsvals[26]={0};   (not a char as in previous examples)
>>> memset(wsvals,0,26);
>>>
>>> Doesn't work.  The code compiles (tcc Windows) with no warnings, but it
>>> doesn't execute.
>> 
>> I presume it does execute.  It just doesn't do what you wanted it to do.
>
> Can't tell what it does.  These are the letter counts /after/ 
> memset(vals,0,26), where vals is an int.  The first one is correct 
> because it's the first count done after initialization.

I can tell you exactly what it does.  It zeros the first 26 *bytes* of
wsvals.  If sizeof (int) == 4 on your system, then that means it zeros
the first 6 elements of the array, and the first half of the seventh
(wsvals[6]), not touching the rest of the array.

You didn't tell us that the declaration (with an initializer) and the
memset call are separate, with memset possibly being called more than
once.  We can only diagnose the code you show us.

If wsvals is initialized to all zeros at the point of definition (with
the `= {0}` initializer), and then only the first quarter of the array
is zeroed on subsequent iterations, then data will accumulate in the
last 3/4 of the array.  I presume the behavior you're seeing is
consistent with that, but I'm not going to take the time to confirm it.
Feel free to do so if you're curious.

[...]

> Why doesn't the compiler recognize the data type of the object being 
> passed to memset, and handle the sizing for you?
> 
> So that memset(arr, v, N) would work for chars and ints.

Because it can't.  memset is a function, not a built-in language
feature.  Its first argument is of type void*, which carries no
information about the type or size of the target object.

That also means that you can use memset() to set every element of a char
array to some non-zero value, but you can't do that with an int array,
because it sets every *byte* to the specified value.  For example:

    char char_array[N];
    int  int_array[N];
    memset(char_array, 0x42, sizeof char_array);
    // sets each element to 0x42
    memset(int_array, 0x42, sizeof int_array);
    // sets each byte to 0x42, which most likely sets
    // each element to 0x42424242

0 is the most commonly used value.

[...]

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
Will write code for food.
void Void(void) { Void(); } /* The recursive call of the void */

[toc] | [prev] | [next] | [standalone]

#147273

DFS <nospam@dfs.com> writes:

> On 10/26/2019 10:58 AM, Manfred wrote:
<cut>
>> and his/her remark about books being unhelpful suggest limited knowledge of C) 
>
>
> Let me show you an example:
>
> Bart posted a modified example of my code.  In it he used memset this way:
>
> #define N_LETTERS 26
> unsigned char sc[N_LETTERS];
> memset(sc, 0, N_LETTERS);
>
>
> A tutorialspoint.com webpage does it similarly:

That's not a book, of course, but I think tutorials were also
mentioned.  I agree with you here.  There are so many awful online
tutorials that it is rarely a good idea to rely on them.

> char str[50];
> memset(str,'$',7);
>
>
> Should be simple, right?

Yes, it should be.  The tutorial is, at best, very misleading.

> So I try it in my code, to replace a small loop:
>
> int wsvals[26]={0};   (not a char as in previous examples)
> memset(wsvals,0,26);

I defence of the (pretty bad) tutorialspoint.com page it describes
memset as a string function (so it's not encouraging its use for int
arrays) and it does give some hints about what memset really does in the
more general case.  It says, for example, that the third parameter is
the number of bytes to be set.

> Doesn't work.  The code compiles (tcc Windows) with no warnings, but
> it doesn't execute.

You are not showing us the real code.  The memset you show here is
harmless as the array is zeroed by the initialisation.  I think the
memset is used elsewhere in the code to get back to the initial state of
wsvals.

Every tutorial about memset should show something like

  int wsvals[26];
  ...
  memset(wsvals, 0, sizeof wsvals);

There is often no need to repeat either the length or the type of the
array.

> Finally on stackoverflow I find this:
> "Note that for memset you have to pass the number of bytes, not the
> number of elements because this is an old C function:
>
> So I do:  memset(wsvals,0,26 * sizeof(int));

but, here, memset(wsvals, 0, sizeof wsvals) works with no change, even
if you switch the type of the array elements to some other integer type.

With dynamically allocated storage, you usually have to do the
multiplication but you can still use a type-agnostic form like this:

  ptr = malloc(n_elements * sizeof *ptr);
  ...
  memset(ptr, 0, n_elements * sizeof *ptr);

There are lots of caveats here (C is not a uniform language), but I'll
leave them to others who might want to point then out -- unless you want
a fuller explanation.

> And it works.
>
> Most of the descriptions on the web say memset "is used to fill a
> block of memory with given/particular value."  No mention of the
> datatype.
>
> Why do I need to also use sizeof() for an int but not a char?

Short answer: because sizeof(char) == 1.  Longer answer: sizeof is
defined to gave the number of char objects that make up the
representation of objects of some type.  C, quite reasonably, calls
these "bytes", but on some very odd machines that can be a bit
surprising.  On the vast majority of systems, int objects are
represented in memory by 2 or 4 consecutive bytes of storage.

-- 
Ben.

[toc] | [prev] | [next] | [standalone]

#147294

On 10/27/19 12:17 AM, DFS wrote:
> On 10/26/2019 10:58 AM, Manfred wrote:
> 
> 
>> There can be much more to be said about the complete program 
> 
> Please, speak up.  Won't hurt my feelings.

No intention to hurt anyone's feelings, my remark was meant as an answer 
to Bart, who was commenting with several points - my intention was more 
like "I know that static_assert won't fix everything" rather than hurt 
feelings.

> 
> 
>> (the OP's memory management mistakes 
> 
> Feel free to show them.  I want to get it as right as possible.
> 
> 
>> and his/her remark about books being unhelpful suggest limited 
>> knowledge of C) 

This part was more "direct", indeed - it could have come out better, I 
admit.
Anyway, you initial code showed confusion between strut words* and 
struct words** pointers, which is something that you would have known if 
you had gone through something like K&R.
One more issue I can point out is how you free()'d memory at the end of 
the program:
> 
>     free(wordrec);
Before explaining my issue with it I have to acknowledge that this 
happens just before program exit, so in this case all memory gets 
deallocated anyway, but the fact that you put the free() call shows that 
your intention is to clean up memory yourself, so here we go. And, more 
importantly, if this were a normal function instead of main(), leaking 
memory would be a relevant matter instead.
Others have pointed out that you allocated wordrec as an array of 
*pointers*, each one of which points to some other malloc()'d chunk of 
memory containing a struct words object, each of which in turns contains 
another pointer 'word' that points to more malloc()'d memory that holds 
the associated string.
I won't repeat that part of this indirection is unnecessary, I will only 
point out that if you free(wordrec) you deallocate only the wordrec 
array, not all memory that is pointed to by the nested pointers. If you 
want to free all memory you should do something like this:

while(0 < wordcnt--)
{
   free(wordrec[wordcnt]->word);
   free(wordrec[wordcnt]);
}
free(wordrec);

Other issues that pop to the eye:
You fgets() from the file with a 30 bytes buffer, but the longest word 
in the dictionary is dichlorodiphenyltrichloroethane which is 31 letters 
long, to which '\n' and the null terminator have to be added, so you are 
reading lines with an insufficient buffer.

You use argv[1] and argv[2] without checking for argc, so if you run the 
program with no arguments you run into UB (most likely access violation).

See below about 'books'

> 
> 
> Let me show you an example:
> 
> Bart posted a modified example of my code.  In it he used memset this way:
> 
> #define N_LETTERS 26
> unsigned char sc[N_LETTERS];
> memset(sc, 0, N_LETTERS);
> 
> 
> A tutorialspoint.com webpage does it similarly:
> char str[50];
> memset(str,'$',7);
> 
> 
> Should be simple, right?
> 
> So I try it in my code, to replace a small loop:
> 
> int wsvals[26]={0};   (not a char as in previous examples)
> memset(wsvals,0,26);
> 
> Doesn't work.  The code compiles (tcc Windows) with no warnings, but it 
> doesn't execute.
> 
> Finally on stackoverflow I find this:
> "Note that for memset you have to pass the number of bytes, not the 
> number of elements because this is an old C function:
> 
> So I do:  memset(wsvals,0,26 * sizeof(int));
> 
> And it works.
> 
> Most of the descriptions on the web say memset "is used to fill a block 
> of memory with given/particular value."  No mention of the datatype.
> 
> Why do I need to also use sizeof() for an int but not a char?
> 
> 
> 

Others have answered, but talking about knowledge and books, the very 
fact that you ask why sizeof() is needed for an int shows limited 
knowledge about C basics; beware, there is obviously nothing wrong about 
this, and it is perfectly OK to ask, but combined with a phrase about 
books being unhelpful it can trigger a remark if the intention is to 
"get it as right as possible"

I left 'tutorials' intentionally out of my remark. As Ben said, there is 
a lot of awful stuff out there.
But in my opinion if someone wants to get C right then reading a good 
book is needed. (Doing it just for fun is a different matter, of course.)

[toc] | [prev] | [next] | [standalone]

#147295

On Saturday, October 26, 2019 at 3:18:15 PM UTC-7, DFS wrote:

> Most of the descriptions on the web say memset "is used to fill a block 
> of memory with given/particular value."  No mention of the datatype.

Why are you using questionable sources for reference about a standard function.  Copies of the standard are readily available (e.g., n1570).  It clearly states

7.24.6.1 The memset function

Synopsis
1 #include <string.h>
void *memset(void *s, int c, size_t n);

Description
2 The memset function copies the value of c (converted to an unsigned char) into
each of the first n characters of the object pointed to by s.

Returns
3 The memset function returns the value of s.

[toc] | [prev] | [standalone]

Page 3 of 3 — ← Prev page 1 2 [3]

Back to top | Article view | comp.lang.c

csiph-web