C programming sprinf issue

Question

I'm using sprintf to print int variable which should display 1^2+2^2+3^2+4^2+5^2+6^2. But my code only prints 1^2+6^2. I don't know why the middle part is missing.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char* formatSeries(int n)
{
  char *tagstr = (char *)malloc(sizeof(char)*n*n);

  int pos = 0 ;
  int k;

  for (k = 1; k <= n; k++)
  {
    pos = sprintf(&tagstr[pos], "%d^2+", k);
  }
  tagstr[strlen(tagstr) - 1] = '\0';

  return tagstr;
}

 void main()
{
  int n = 6;
  printf("%s \n", formatSeries(n));
}

[Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). — Sourav Ghosh, Apr 12 '17 at 06:35
Yep - @a3f has it. Also this line `tagstr[strlen(tagstr) - 1] = '\0';` is....wrong...as well as unnecessary. Also the initial size of your array is not big enough until about n=4 — LoztInSpace, Apr 12 '17 at 06:39
Also, why is your loop going from 1 to n instead of 0 to n-1? — NoseKnowsAll, Apr 12 '17 at 06:43
@NoseKnowsAll because the values being printed warrant it. It has nothing to do with indexing. — WhozCraig, Apr 12 '17 at 06:43
@LoztInSpace that line has purpose (trim the last `+` from the string), but it could be better done with simply `tagstr[pos-1] = 0;` once the fix from a3f is deployed. — WhozCraig, Apr 12 '17 at 06:44

score 2 · Accepted Answer · answered Apr 12 '17 at 06:56

There are two main issues in your code; the one - as already mentioned in the comments - is that you do not increase pos but reassign it again and again with (almost) the same value. Hence, after the first iteration, you will constantly write to a position at 4 (or 5 if k would become >=10 and the numbers would thereby get more digits),

Second, your malloc will not work for small ks, since each string like "+1^2" takes at least 4 characters; so if you size the buffer with n * n, then the buffer will be too small if n is <= 3, and (probably much) too big if n becomes larger.

I'd suggest to assume a maximum range of n (let's say up to 7 digits) and dimension the buffer accordingly.

A minor thing (and not essential) is that you probably could avoid writing superfluous + at all in that you use a different format string for the first iteration; just do show the idea, really not essential:

char* formatSeries(int n)
{
    const size_t maxSize = n * (7+3) + 1;

    char *tagstr = (char *)malloc(sizeof(char)*maxSize);

    int pos = 0 ;
    int k;
    for (k = 1; k <= n; k++)
    {
        const char* formatStr = (k==1) ? "%d^2" : "+%d^2";
        pos += sprintf(&tagstr[pos], formatStr, k);

    }
    return tagstr;
}

C programming sprinf issue

1 Answers1