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


Groups > comp.lang.c > #162002

Re: Allocate length of word vs fixed length

Message-Id <pcmhsh-pp1.ln1@aretha.foo>
From Peter 'Shaggy' Haywood <phaywood@alphalink.com.au>
Subject Re: Allocate length of word vs fixed length
Newsgroups comp.lang.c
Date 2021-07-20 11:44 +1000
References <U3iJI.61694$VU3.5811@fx46.iad>

Show all headers | View raw


Groovy hepcat dfs was jivin' in comp.lang.c on Tue, 20 Jul 2021 02:50
am. It's a cool scene! Dig it.

> Loaded a list of words into an array.
> 
> The 370103 words came from https://github.com/dwyl/english-words
> file = words_alpha.txt
> 
> Tested a couple memory allocation 'strategies' during loading:
> 
> 1. allocate the strlen() of each word
> 2. allocate a fixed length (len of longest word = 32)
> 
> I figured getting strlen(word) each time would be slower than
> allocating a fixed amt of memory, but that wasn't the case.
> 
> Strategy 2 is significantly slower, and uses nearly 3x the memory.
> 
> Also, does anyone have any 'tricks' to make such a file load routine
> faster/more efficient?  Thanks

  That all depends on what you're actually trying to do. Reading in an
array of words is a means to some end, not an end in itself. What do
you want to do with these words?

> ===========================================================
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <ctype.h>
> 
> #define maxlen 32

  If the longest word is 32 letters, then maxlen should be 33 to allow
for the string's terminating null character. (Ike advised you about
allowing for this null in his response to you, but you didn't seem to
understand. See below.) The way you're reading in words, your 32 letter
word will be truncated to 31 letters, and the 32nd letter will wind up
being the next word.

> //removes trailing isspaces and quote marks
> char *rtrim(char *str)
> {
> int len = strlen(str);
> while(len>0 && (isspace(str[len-1]) || str[len-1] == '\"'))   {len--;}
> str[len] = '\0';
> return str;
> }


  Why is this (above) function needed? Does your file contain white
space (other than newlines) and quotes? What about such characters at
the start of a word?
  This could probably be done by some kind of script before feeding the
file to your program.

> int main(int argc, char *argv[])
> {
> //vars

  A point on style: it's a bad idea to use pointless comments like this.
Comments should explain the working and reasoning for code that's not
immediately obvious. Comments that don't do that just clutter the code
and can, in many cases, make it harder to comprehend.

> int  i = 0, lines = 0;

  Another word or two on style here: I'd make i and lines type size_t.
The point of size_t is to represent sizes (lines, in this case), so it
is good to use it for that. And the counter (i) is used to access
elements over the length of the array, so to my mind it makes sense to
use the same type for that (though no doubt others may disagree).
  Also, you aren't using i until way below here. It might be better to
declare (or at least "initialise") it just before using it.

> char word[maxlen] = "";
> char wordin[maxlen] = "";

  Initialising these is pointless, since you're only overwriting them
anyhow.

> //open file, count lines
> FILE *fwords = fopen(argv[1],"r");
> while(fgets(wordin,sizeof wordin,fwords)!=NULL) {lines++;}
> 
> //Geany adds a line feed to the very last line, so the true
> //line count is overstated

  Now that's a better comment. :)

> lines -= 1;
> 
> //mem
> char **words = malloc(sizeof(char*) * lines);

  Another style point: it is bad style to hard wire the type in a
malloc() call. This can lead to problems in case the type of the thing
being allocated changes. It is better to use something like the
following:

type *ptr = malloc(sizeof *ptr * num);

That way the size is always right for the thing being allocated. I know
Ike touched on this too, but it bears repeating.
  Also, always check the return from functions that can fail, like
malloc(). This is vital. Never leave out this important step.

> //add words to array
> int memoption = atoi(argv[2]);

  Always check that you have enough command line args when attempting to
use them. Again, this is vital. It's arguably more important than
checking that malloc() succeeded, because command line args usually
come from a human user; and you know how unpredictable those creatures
can be!
  Also, what about argv[1]? You're not going to use that too?

> int memlen = 0, memtotal = 0;
> rewind(fwords);

  Again, check that this succeeded. Use ferror() for this.

> while(fgets(wordin,sizeof wordin,fwords)!=NULL)
> {
> strcpy(word,rtrim(wordin));
>
> //different memory allocations
> if(memoption==1)
> {memlen=strlen(word);}

  Now, about what I alluded to above (and Ike told you about), this is a
classic "off by one" error. To store a 5 letter word (for example) as a
string, you need 6 bytes. strlen() will return 5, meaning you need to
add 1 here. Remember, strlen() returns the length of the string *not*
including the terminating null character. You need to add 1.

> else
> {memlen=maxlen;}
> 
> words[i] = malloc(sizeof(char*) * memlen);

  I think Ike also mentioned the problem here. Remember as a general
rule, allocate the size of the thing being allocated, not a hard wired
type. But when you're just getting space for a string, it's okay to
just leave out the type altogether. You just want to allocate enough
bytes for the string (memlen bytes, in this case).

words[i] = malloc(memlen);

And, again, check that it succeeded.

> memtotal += memlen;
> 
> strcpy(words[i],word);
> 
> //printf("%d. '%s' ",i,words[i]);
> i++;
> }
> 
> fclose(fwords);
> printf("\nloaded %d words\n",i-1);
> printf("\nFirst and Last words = '%s', '%s'\n",words[0],words[i-1]);
> printf("malloc option %d, total memory %d\n",memoption,memtotal);
> free(words);

  Here you're freeing the overall array, but you're not freeing all the
smaller arrays (strings).

> return(0);

  You don't need the parentheses around the return value here; return is
not a function.

> }

-- 


----- Dig the NEW and IMPROVED news sig!! -----


-------------- Shaggy was here! ---------------
              Ain't I'm a dawg!!

Back to comp.lang.c | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Allocate length of word vs fixed length dfs <nospam@dfs.com> - 2021-07-19 12:50 -0400
  Re: Allocate length of word vs fixed length Bart <bc@freeuk.com> - 2021-07-19 19:01 +0100
    Re: Allocate length of word vs fixed length dfs <nospam@dfs.com> - 2021-07-19 14:29 -0400
      Re: Allocate length of word vs fixed length Bart <bc@freeuk.com> - 2021-07-19 19:49 +0100
        Re: Allocate length of word vs fixed length dfs <nospam@dfs.com> - 2021-07-19 18:01 -0400
          Re: Allocate length of word vs fixed length Bart <bc@freeuk.com> - 2021-07-19 23:25 +0100
      Re: Allocate length of word vs fixed length scott@slp53.sl.home (Scott Lurndal) - 2021-07-19 20:00 +0000
        Re: Allocate length of word vs fixed length Branimir Maksimovic <branimir.maksimovic@gmail.com> - 2021-07-19 23:29 +0000
      Re: Allocate length of word vs fixed length Real Troll <real.troll@trolls.com> - 2021-07-20 00:30 +0100
        Re: Allocate length of word vs fixed length DFS <nospam@dfs.com> - 2021-07-19 20:40 -0400
          Re: Allocate length of word vs fixed length Real Troll <real.troll@trolls.com> - 2021-07-20 17:00 +0000
      Re: Allocate length of word vs fixed length Kaz Kylheku <563-365-8930@kylheku.com> - 2021-07-24 01:07 +0000
  Re: Allocate length of word vs fixed length David Brown <david.brown@hesbynett.no> - 2021-07-19 21:35 +0200
    Re: Allocate length of word vs fixed length dfs <nospam@dfs.com> - 2021-07-19 18:02 -0400
  Re: Allocate length of word vs fixed length Ike Naar <ike@rie.sdf.org> - 2021-07-19 21:11 +0000
    Re: Allocate length of word vs fixed length dfs <nospam@dfs.com> - 2021-07-19 18:03 -0400
      Re: Allocate length of word vs fixed length Ike Naar <ike@sdf.org> - 2021-07-20 05:28 +0000
  Re: Allocate length of word vs fixed length dfs <nospam@dfs.com> - 2021-07-19 18:21 -0400
    Re: Allocate length of word vs fixed length Bart <bc@freeuk.com> - 2021-07-19 23:29 +0100
      Re: Allocate length of word vs fixed length dfs <nospam@dfs.com> - 2021-07-19 18:40 -0400
  Re: Allocate length of word vs fixed length Peter 'Shaggy' Haywood <phaywood@alphalink.com.au> - 2021-07-20 11:44 +1000
  Re: Allocate length of word vs fixed length Real Troll <real.troll@trolls.com> - 2021-07-20 22:15 +0000
    Re: Allocate length of word vs fixed length dfs <nospam@dfs.com> - 2021-07-20 20:22 -0400
      Re: Allocate length of word vs fixed length dfs <nospam@dfs.com> - 2021-07-21 23:23 -0400

csiph-web