[vsipl++] [patch] SSAR Make targets

[Stefan Seefeld]

Don McCoy wrote:

> Modified: csl/vpp/trunk/apps/ssar/GNUmakefile
> ==============================================================================

> +# The default precision is single (double may also be used)
> +precision = single
> +
> +ifeq ($(precision),double)
> +ref_image_base = ref_image_dp
> +ssar_type = SSAR_BASE_TYPE=double
> +else
> +ref_image_base = ref_image_sp
> +ssar_type = SSAR_BASE_TYPE=float
> +endif
> +

What is the effect of "ssar_type = SSAR_BASE_TYPE=double" in a (GNU) makefile ?
We usually use the ':=' assignment operator whenever possible. The only reason
not to do that is if we have to evaluate the assinment lazily (i.e. because it
refers to an expression of variables with yet unknown values).



> Index: apps/ssar/load_save.hpp
> ===================================================================
> --- apps/ssar/load_save.hpp	(revision 0)
> +++ apps/ssar/load_save.hpp	(revision 0)
> @@ -0,0 +1,114 @@
> +/* Copyright (c) 2006 by CodeSourcery.  All rights reserved. */
> +
> +/** @file    load_save.hpp
> +    @author  Don McCoy
> +    @date    2006-10-26
> +    @brief   Extensions to allow type double to be used as the view
> +             data type while using float as the storage type on disk.
> +*/
> +
> +#ifndef LOAD_SAVE_HPP
> +#define LOAD_SAVE_HPP
> +
> +#include <vsip_csl/load_view.hpp>
> +#include <vsip_csl/save_view.hpp>
> +
> +using namespace vsip_csl;

Please never, ever, use a 'using namespace' declaration in global scope in a header.
I understand that here it isn't dangerous since this is not a library, but it is still
confusing, and error-prone.

> Index: apps/ssar/diffview.cpp
> ===================================================================
> --- apps/ssar/diffview.cpp	(revision 0)
> +++ apps/ssar/diffview.cpp	(revision 0)
> @@ -0,0 +1,110 @@
> +/* Copyright (c) 2006 by CodeSourcery.  All rights reserved. */
> +
> +/** @file    diffview.cpp
> +    @author  Don McCoy
> +    @date    2006-10-29
> +    @brief   Utility to compare VSIPL++ views to determine equality
> +*/
> +
> +#include <iostream>
> +#include <stdlib.h>
> +
> +#include <vsip/initfin.hpp>
> +#include <vsip/math.hpp>
> +
> +#include <vsip_csl/load_view.hpp>
> +#include <vsip_csl/save_view.hpp>
> +#include <vsip_csl/error_db.hpp>
> +
> +
> +using namespace vsip;
> +using namespace vsip_csl;
> +using namespace std;
> +
> +
> +enum data_format_type
> +{
> +  COMPLEX_VIEW = 0,
> +  REAL_VIEW,
> +  INTEGER_VIEW
> +};
> +
> +void compare(data_format_type format, 
> +  char const* infile, char const* ref, length_type rows, length_type cols);
> +
> +int
> +main(int argc, char** argv)
> +{
> +  vsip::vsipl init(argc, argv);
> +
> +  if (argc < 5 || argc > 6)
> +  {
> +    fprintf(stderr, "Usage: %s [-rn] <input> <reference> <rows> <cols>\n", 
> +      argv[0]);
Why not


     std::cerr << "Usage: " << arv[0] << "[-rn] <input> <reference> <rows> <cols>" << std::endl;

?

Now we are using both, std::iostreams, as well as stdio. I think we should use one consistently.
That may even reduce the size of the program.

Thanks,
		Stefan

-- 
Stefan Seefeld
CodeSourcery
stefan at codesourcery.com
(650) 331-3385 x718