| Re: [AD] Current SVN OS X issues |
[ Thread Index |
Date Index
| More lists.liballeg.org/allegro-developers Archives
]
On 2010-01-27, Evert Glebbeek <eglebbk@xxxxxxxxxx> wrote:
> On 27 Jan 2010, at 17:43 , Peter Hull wrote:
> > I ran clang again, results are here
> > http://homepage.ntlworld.com/valleyway/clang.zip
>
> WOW.
> I ran the static analyser through make (rather than xcodebuild) and for me it only found 64 potential bugs. I'll have to make my way through your list.
> Most of those it found for me are actually very innocuous, just some variebles being assigned values that are never used. They should probably cleared up because they clutter the code but they don't actually hurt.
> However, there are, I think, a couple that are clearly bugs.
> For instance, there is stuff like
>
> 757 bool_Bool al_save_bmp_stream(ALLEGRO_FILE *f, ALLEGRO_BITMAP *bmp)
> 758 {
> 759 int bfSize;
> 760 int biSizeImage;
> 761 int depth;
> 762 int bpp;
> 763 int filler;
> 764 int i, j;
> 765 int w, h;
> 766 ALLEGRO_LOCKED_REGION *lr;
> 767 ASSERT(f);
> 768 ASSERT(bmp);
> 769
> 770 w = al_get_bitmap_width(bmp);
> 771 h = al_get_bitmap_height(bmp);
> 772
> 773 depth = al_get_pixel_format_bits(al_get_bitmap_format(bmp));
>
> Value stored to 'depth' is never read
> 774 bpp = 24;
> 775 filler = 3 - ((w * (bpp / 8) - 1) & 3);
>
> where it looks as though line 773 should read bpp = al_get_pixel_format_bits() and line 774 shouldn't be there at all.
No, the main part of the routine only knows how to write 24-bit .BMP
files. All the leftover code concerning 8-bit .BMPs should be deleted
and a comment added. The source bitmap format is irrelevant as we call
al_get_pixel (slow?) so line 773 should be deleted, too.
> I'm also not sure how this is supposed to work:
> 560 static ALLEGRO_PATH *fs_stdio_get_current_directory(void)
> 561 {
> 562 char tmpdir[PATH_MAX1024];
> 563 char *cwd = getcwd(tmpdir, PATH_MAX1024);
> 564 size_t len;
> 565 if (!cwd) {
> 566 al_set_errno(errno(*__error()));
> 567 return NULL((void*)0);
> 568 }
> 569 len = strlen(cwd);
>
> Value stored to 'len' is never read
> 570
> 571 return al_create_path_for_directory(tmpdir);
> 572 }
>
> since it seems to me that the value of cwd is never actually used so
> trying to read the current directory through Allegro's path API should
> return garbage, can someone verify this?
The code is okay. On success, the path will be stored in tmpdir(==cwd).
It would be clearer if line 571 used cwd.
Peter