Re: [PATCH] ntdll: Increase size of buffer used to read lines of /proc/cpuinfo

[Vitaliy Margolen]

On 11/24/2010 12:23 PM, [e-mail] wrote:
> From: James Eder
> - while (fgets(line,200,f) != NULL)
> - while (fgets(line,450,f) != NULL)

You might as well then change this to "sizeof(line)".

Vitaliy.

[PATCH] ntdll: Increase size of buffer used to read lines of /proc/cpuinfo

[James McKenzie]

On 11/24/10 6:56 PM, Vitaliy Margolen wrote:
> On 11/24/2010 12:23 PM, [e-mail] wrote:
> > From: James Eder
> > - while (fgets(line,200,f) != NULL)
> > - while (fgets(line,450,f) != NULL)
>
> You might as well then change this to "sizeof(line)".

Just for my edification, is there not a better way then setting the 
variable line to a flexible length for this purpose.  This might prevent 
'growing' the variable over time.

James McKenzie

[PATCH] ntdll: Increase size of buffer used to read lines of /proc/cpuinfo

[Vitaliy Margolen]

On 11/24/2010 07:19 PM, James McKenzie wrote:
> On 11/24/10 6:56 PM, Vitaliy Margolen wrote:
> > On 11/24/2010 12:23 PM, [e-mail] wrote:
> > > From: James Eder
> > > - while (fgets(line,200,f) != NULL)
> > > - while (fgets(line,450,f) != NULL)
> >
> > You might as well then change this to "sizeof(line)".
>
> Just for my edification, is there not a better way then setting the variable
> line to a flexible length for this purpose.

Unless I didn't understand your question - you can't set a buffer to a 
"variable length". You have to provide fgets() a size of the buffer so it 
can read at most that many characters -1 for terminating \0.

Vitaliy.

[PATCH] ntdll: Increase size of buffer used to read lines of /proc/cpuinfo

[Damjan Jovanovic]

On Fri, Nov 26, 2010 at 6:56 AM, Vitaliy Margolen
 wrote:
> On 11/24/2010 07:19 PM, James McKenzie wrote:
> > On 11/24/10 6:56 PM, Vitaliy Margolen wrote:
> > > On 11/24/2010 12:23 PM, [e-mail] wrote:
> > > > From: James Eder
> > > > - while (fgets(line,200,f) != NULL)
> > > > - while (fgets(line,450,f) != NULL)
> > >
> > > You might as well then change this to "sizeof(line)".
> >
> > Just for my edification, is there not a better way then setting the
> > variable
> > line to a flexible length for this purpose.
>
> Unless I didn't understand your question - you can't set a buffer to a
> "variable length". You have to provide fgets() a size of the buffer so it
> can read at most that many characters -1 for terminating \0.
>
> Vitaliy.

So read lines dynamically instead:

static BOOL next_line(FILE *file, char **line, int *size)
{
    int pos = 0;
    char *cr;
    if (*line == NULL)
    {
        *size = 4096;
        *line = HeapAlloc(GetProcessHeap(), 0, *size);
    }
    while (*line != NULL)
    {
        if (fgets(&(*line)[pos], *size - pos, file) == NULL)
        {
            HeapFree(GetProcessHeap(), 0, *line);
            *line = NULL;
            if (feof(file))
                return TRUE;
            return FALSE;
        }
        pos = strlen(*line);
        cr = strchr(*line, '\n');
        if (cr == NULL)
        {
            char *line2;
            (*size) *= 2;
            line2 = HeapReAlloc(GetProcessHeap(), 0, *line, *size);
            if (line2)
                *line = line2;
            else
            {
                HeapFree(GetProcessHeap(), 0, *line);
                *line = NULL;
            }

}
        else
        {
            *cr = 0;
            return TRUE;
        }
    }
    return FALSE;
}

Damjan

[PATCH] ntdll: Increase size of buffer used to read lines of /proc/cpuinfo

[James McKenzie]

On 11/26/10 12:15 AM, Damjan Jovanovic wrote:
> On Fri, Nov 26, 2010 at 6:56 AM, Vitaliy Margolen
>   wrote:
> > On 11/24/2010 07:19 PM, James McKenzie wrote:
> > > On 11/24/10 6:56 PM, Vitaliy Margolen wrote:
> > > > On 11/24/2010 12:23 PM, [e-mail] wrote:
> > > > > From: James Eder
> > > > > - while (fgets(line,200,f) != NULL)
> > > > > - while (fgets(line,450,f) != NULL)
> > > >
> > > > You might as well then change this to "sizeof(line)".
> > >
> > > Just for my edification, is there not a better way then setting the
> > > variable
> > > line to a flexible length for this purpose.
> >
> > Unless I didn't understand your question - you can't set a buffer to a
> > "variable length". You have to provide fgets() a size of the buffer so it
> > can read at most that many characters -1 for terminating \0.
> >
> > Vitaliy.
>
> So read lines dynamically instead:

This is exactly what I was trying to get at.  Thank you Damjan.

James McKenzie

[PATCH] ntdll: Increase size of buffer used to read lines of /proc/cpuinfo

[James McKenzie]

On 11/25/10 9:56 PM, Vitaliy Margolen wrote:
> On 11/24/2010 07:19 PM, James McKenzie wrote:
> > On 11/24/10 6:56 PM, Vitaliy Margolen wrote:
> > > On 11/24/2010 12:23 PM, [e-mail] wrote:
> > > > From: James Eder
> > > > - while (fgets(line,200,f) != NULL)
> > > > - while (fgets(line,450,f) != NULL)
> > >
> > > You might as well then change this to "sizeof(line)".
> >
> > Just for my edification, is there not a better way then setting the 
> > variable
> > line to a flexible length for this purpose.
>
> Unless I didn't understand your question - you can't set a buffer to a 
> "variable length". You have to provide fgets() a size of the buffer so 
> it can read at most that many characters -1 for terminating \0.

Clarification:  If we  know the length of the input should we not set up 
the buffer to that length?  If we don't know should we not set it to a 
maximum expected length that is a known value within the system?

Pseudo code for known case:

Length = # of characters in existing variable
buffer [Length +1]

use buffer...

Pseudo code for unknown case:

MaxLengthofCharacterString = 1024
buffer [MaxLengthofCharacterString]

use buffer...

Gets away from 'magic numbers' like 250 and 450.

James McKenzie