[GRASS-dev] r.colors: rules file list unsorted

Paul Kelly paul-grass at stjohnspoint.co.uk
Mon Jan 29 06:07:05 EST 2007


I've added the updated version to CVS now. There are two new functions:
G_ls() prints a directory listing in column format to the specified 
stream.
G__ls() stores a list of the files in a directory in an array. (It is a 
lower level function used by G_ls().)

I've also updated lib/init/set_data.c to use these two new functions for 
listing locations and mapsets during the text-based startup. Hopefully 
they will be useful in many other places too, both to replace usage of 
system("ls") etc. and also to reduce cloned code for directory listing.

On Mon, 29 Jan 2007, Paul Kelly wrote:

> On Mon, 29 Jan 2007, Glynn Clements wrote:
>
>> 
>> Paul Kelly wrote:
>> 
>>>>> A library function should probably return the entries as an array of
>>>>> strings (char **) rather than write them to a stream. The caller can
>>>>> deal with writing them, if that's desired.
>>>> 
>>>> That's how it's done in the patched r.colors. If it must be turned
>>>> into a library function, where would it go? What would its name be?
>>>> I'm willing to do it (and also change any system("ls")'s remaining).
>>> 
>>> I've just finished it - see attached - planning on committing it as
>>> lib/gis/ls.c tomorrow after some more polishing if no objections.
>>> 
>>> The way I've done it there are 3 functions:
>>> 
>>> G__ls() reads the directory and stores the entries in an array.
>> 
>>> G_ls() prints the listing to a specified stream, one entry per line.
>> 
>> Do we really need a library function for a for/fprintf loop?
>
> Probably not. I was including it for the sake of completeness really, but 
> actually I haven't yet seen anywhere it would be useful. Most modules that 
> call system("ls") appear to rely on column output. Maybe I will make G_ls() 
> do the column output and get rid of the one-file-per-line version.
>
>>>> The parser uses column break with a hardcoded value. We certainly
>>>> could use a global function for determining column width.
>>> 
>>> I used the ioctl method. Works for me on Linux but not on Windows - have
>>> disabled the functionality there and it just uses hard-coded 80 characters
>>> for now. But it does work nicely on Linux as far as I can see.
>> 
>>> #ifndef __MINGW32__
>>> #  include <sys/ioctl.h>
>>> #endif
>> 
>> This should use HAVE_SYS_IOCTL_H from <grass/config.h>.
>
> Right - didn't know about that, have changed it.
>
>> 
>>> #ifndef __MINGW32__
>>>     if (isatty(fileno(stream)))
>>>     {
>>>         /* Determine screen_width if output is a terminal */
>>> 	struct winsize size;
>>> 
>>> 	ioctl(fileno(stream), TIOCGWINSZ, (char *) &size);
>>> 	screen_width = size.ws_col;
>>>     }
>>> #endif
>> 
>> The code should be conditionalised upon "#ifdef TIOCGWINSZ". That
>> should ensure that we don't try to use it if it isn't defined. Even if
>> the platform has it, it might not be defined in <sys/ioctl.h>. FWIW,
>> SUSv2 only specifies ioctl() with regard to STREAMS (which is an
>> optional feature).
>
> OK, have done that.
>
>> Also, the return value from ioctl() should be checked. Having done
>> that, you can dispense with the isatty() check; call ioctl()
>> unconditionally and only use the result if it succeeds.
>
> Yes, had already changed that in my local version (the check for the return 
> value of ioctl) BUT, my idea was keep the isatty check there
> as I thought other platform-specific versions of the terminal width check 
> could be included in the future within the isatty if-construct. Not sure 
> though now if it's worth bothering with. I suppose 80 is a good default.
>
>> However, I'm still unsure whether we might have problems due to
>> requiring some unspecified header in order to get "struct winsize".
>> Bear in mind that this is libgis, and if libgis fails to compile so
>> does everything else. Personally, I'd be inclined to just hard-code a
>> setting of 80 columns.
>
> Hmmm, everywhere I've seen on the web seems to suggest struct winsize is 
> defined in sys/ioctl.h. And the mention of winsize is already conditionalised 
> on TIOCGWINSZ being defined; I should think we're pretty safe, no?
>
> _______________________________________________
> grass-dev mailing list
> grass-dev at grass.itc.it
> http://grass.itc.it/mailman/listinfo/grass-dev
>




More information about the grass-dev mailing list