LINUX.ORG.RU
решено Форум — Development  

Просьба проверить учебную программу на языке Си на наличие ошибок - 2

 , ,


0

0

Порядок сборки программы: 

cc -Wall -o writeln writeln.c

Порядок запуска:

./writeln аргумент1 аргумент2 ... аргументN

Программа должна добавить к файлу test.txt такую строку:

аргумент1 аргумент2 ... аргументN\n

В случае обнаружения ошибок, недочетов в программе просьба сообщить.
Спасибо.

Исходный код файла «writeln.h»:

const char FILE_NAME[] = "test.txt";

int print_help( char * prog_name );
int fopen_error( void );
int fwrite_error( FILE * stream );

Исходный код файла «writeln.c»:

#include <stdio.h>
#include "writeln.h"

int main( int argc , char * argv[] ) {
  FILE * stream;
  int chr , i;

  if ( argc == 1 )
    return print_help( argv[ 0 ] );

  stream = fopen( FILE_NAME , "a" );

  if ( stream == NULL )
    return fopen_error();

  for( i = 1 ; i < argc ; i++ ) {
    if ( fputs( argv[ i ] , stream ) == EOF )
      return fwrite_error( stream );

    if ( i == argc-1 )
      chr = '\n';
    else
      chr = ' ';

    if ( fputc( chr , stream ) == EOF )
      return fwrite_error( stream );
  }

  fclose( stream );
  return 0;
}

int print_help( char * prog_name ) {
  fprintf( stderr , "Usage: %s Text of the message\n" , prog_name );
  return 1;
}

int fopen_error( void ) {
  fprintf( stderr , "Can't open file '%s'.\n" , FILE_NAME );
  return 2;
}

int fwrite_error( FILE * stream ) {
  fclose( stream );
  fprintf( stderr , "Error writing file '%s'.\n" , FILE_NAME );
  return 3;
}
Deleted
← Как вставить в приложение вывод окна другого процесса?
По ссылке или по значению? → 

const char FILE_NAME[] = "test.txt";

Зачем делать const char[] в хедере? В каждый сорс пойдет по глобальному екземпляру FILE_NAME, при линковке получится error: multiple definition of FILE_NAME.

int print_help( char * prog_name )

Вот как раз prog_name имеет смысл делать глобальной, и перед парсингом argv[] делать prog_name=argv[0]. Кстати, где ты увидел такой стиль: f( void ). Надо f(void), лишние пробелы там ни к чему.

mtk ★
()
Последнее исправление: mtk (всего исправлений: 2)

А зачем здесь вообще writeln.h?

anonymous
()
Ответ на: комментарий от theNamelessOne

theNamelessOne

FILE_NAME[] засунь в writeln.c, а в хедере сделай extern.

Согласен, так как добавление к заголовочному файлу строки 

extern const char FILE_NAME[];
обеспечивает видимость константы в дополнительных модулях исходного кода, к которым подключен «writeln.h».

Deleted
()

Вот как я бы переделал (дело вкуса): 

static char *prog_name;

int main( int argc , char *argv[] ) {
  int ret = 0;
  FILE * stream = NULL;
  int chr , i;
  
  prog_name = argv[0];
  if (argc == 1) {
    usage();
    exit(1);
  }

  stream = fopen(FILE_NAME , "a");
  if (stream == NULL) {
      fprintf( stderr , "Can't open file '%s'.\n" , FILE_NAME );
      return 2;
  }

  ret = 3;
  for(i = 1 ; i < argc ; i++) {
    if (fputs(argv[i] , stream) == EOF)
      goto err;

    if (i == argc-1)
      chr = '\n';
    else
      chr = ' ';

    if (fputc(chr, stream) == EOF)
      goto err;
  }
  ret = 0;

err:
  if (stream)
    fclose(stream);
  if (ret != 0)
    fprintf(stderr , "Error writing file '%s'.\n" , FILE_NAME);
  return ret;
}

void usage(void) {
  fprintf(stderr , "Usage: %s Text of the message\n" , prog_name);
}
Получается, благодаря ret + err вообще нет нужды в функциях с кодами возврата.

Чувствую, щас царь придет и все обосрет, если его снова не забанили.

То есть, ошибок нет, есть отсутствие единого стиля, что важнее, так как ошибки всегда можно в дебаггере отловить, а вкус вырабатывается годами.

mtk ★
()
Ответ на: комментарий от mtk

mtk

Зачем делать const char[] в хедере? В каждый сорс пойдет по глобальному екземпляру FILE_NAME, при линковке получится error: multiple definition of FILE_NAME.

Благодарю за подробное разъяснение.

mtk

Вот как раз prog_name имеет смысл делать глобальной, и перед парсингом argv[] делать prog_name=argv[0].

Согласен, так как исключается необходимость передавать указатель argv[0] в другие функции в качестве аргумента.

Deleted
()
Ответ на: комментарий от anonymous

Это сейчас так, после переделок логика изменится, а проверка if (stream) в конце останется валидной. То же самое касается exit(), если отрефакторить все в функцию, то exit останется в силе. Как я говорил, дело вкуса.

mtk ★
()
Последнее исправление: mtk (всего исправлений: 1)

код ошибок

#define ERROR_PONY1 0
#define ERROR_PONY2 1

//бла-бла тут код и завершаем программу с ошибкой
return ERROR_PONY1;
fornlr ★★★★★
()
Последнее исправление: fornlr (всего исправлений: 2)
Ответ на: комментарий от fornlr

Уж лучше тогда 

typedef enum {
  ERROR_NOPONIES,
  ERROR_PONY1,
  ERROR_PONY2,
  ERROR_PONY3,
} TestError;
Тогда можно делать TestError ret = ERROR_NOPONIES;, не опасаясь получить левое значение ошибки. Тем более, удобней в дебаггере получить символьное представление ошибки вместо цифры (в случае define).

mtk ★
()
Ответ на: комментарий от anonymous

Если очень постараться, можно и свой _main() написать.

mtk ★
()

Почему бы не вынести печать '\n' на после цикла?

kike
()
Ответ на: комментарий от Deleted

обеспечивает видимость константы в дополнительных модулях исходного кода, к которым подключен «writeln.h».

Дело не в этом, это нужно для того, чтобы при линковке разных модулей не было ошибки.

theNamelessOne ★★★★★
()

разбирать код детально не буду, только комментарий по идеалогии: любой код пишется так чтоб потом его ещё где-нить поиспользовать. Поэтому задача «вывести аргументы в файл одной строкой» оформляется как функциия и не место этому в main. То есть должна быть int some_name(FILE *dest,size_t arr_size, char **arr); Потом ещё пригодится - например добавите разделитель записей и терминатор строки и будете генерять CSV.

порождение отдельных xxx_error на мой взгляд лишняя и вредная сучность. Ошибки стандартные, код ошибок в errno, описание в strerror(), чтобы вывести в журнал сообщение достаточно одной функции/макроса. А хардкодить закрытие потока при любой ошибке записи - не слишком полезная функция. К тому-же пресловутые xxx_error (если вы уж решились из делать) не имеют логического отношения к основной задаче и не место им в «writeln.h» и соответсвующим «writeln.c»

MKuznetsov ★★★★★
()
Ответ на: комментарий от MKuznetsov

любой код пишется так чтоб потом его ещё где-нить поиспользовать

wrong

kike
()

1:

 if ( argc == 1 )
    return print_help( argv[ 0 ] );
2:

for( i = 1 ; i < argc ; i++ ) {
    if ( fputs( argv[ i ] , stream ) == EOF )
      return fwrite_error( stream );

    if ( i == argc-1 )
      chr = '\n';
    else
      chr = ' ';

    if ( fputc( chr , stream ) == EOF )
      return fwrite_error( stream );
  }
Как уже сказали, в первом участке надо <=1

Если дошёл до 2-го участка, то argv[1] - существует, а значит ты можешь вывести argv[1], потом в цикле ' '+argv[ i], и в конце '\n'

AlexVR ★★★★★
()

проверяем домашку? ок.

юз perror, люк

anonymous
()
Ответ на: комментарий от anonymous

Вот черт! Надо будет иметь это в виду! А то я иногда просто так и проверяю ☹

Eddy_Em ☆☆☆☆☆
()
19 мая 2015 г.
Ответ на: комментарий от anonymous

Однако...

Я по наитию всегда проверял через >, теперь знаю, почему :)

// hobbit, с чужого компа.

anonymous
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.
← Как вставить в приложение вывод окна другого процесса?
Development
По ссылке или по значению? →